On 9/22/2022 6:04 AM, Jeff King wrote: > Running t9210 with the tip of master triggers a problem with UBSan: > > $ make SANITIZE=undefined > [...] > $ cd t > $ ./t9210-scalar.sh -v -i > [...] > read-cache.c:1886:46: runtime error: member access within misaligned address 0x7f7c09bf7055 for type 'struct ondisk_cache_entry', which requires 4 byte alignment > 0x7f7c09bf7055: note: pointer points here > 33 2e 74 00 63 2c 31 42 17 3f 49 72 63 2c 31 42 17 3f 49 72 00 00 fe 01 02 60 06 4d 00 00 81 a4 > ^ > > Now here's the interesting part. We do carefully read most of the data > out of the struct with get_be16(), which should handle alignment (we > have to do so because that on_disk_cache_entry is literally just a cast > from an mmap'd buffer). But the line in question is just: > > const uint16_t *flagsp = (const uint16_t *)(ondisk->data + hashsz); > > It's not even reading anything, but just computing an offset within the > struct. I don't think that line in particular is to blame. If I use an > older version of Git that predates it on the same repo generated by > t9210, I get a similar error for a different line. > > Another thing to note is that the command which fails isn't scalar > itself! It's just vanilla "git add -- loose.t". But it's curious that we > never saw this alignment problem before. I wonder if the scalar-cloned > repository has some index options turned on that trigger the issue. > > I didn't dig further. It's obviously new in v2.38.0-rc1, but I'm not > sure it's a show-stopper. It _might_ have been there all along, and is > just now surfacing. Or it might be in an existing experimental feature > that just wasn't exercised enough in the tests. Or if it really is new > in scalar, then it will only hurt people using scalar, which didn't > exist before. So I don't think it's a regression in the strictest sense, > but it might be nice to get a more accurate understanding of the problem > before the release. Interesting find! Here are the index-related settings that Scalar sets as of -rc1: * core.preloadIndex=true * index.threads=true * index.version=4 My gut feeling is that index.version=4 might be the culprit. I thought we tested GIT_TEST_INDEX_VERSION=4 in some CI modes, but apparently we do not. Do you get the same error in other tests with that environment variable? Thanks, -Stolee
