git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Victoria Dye <vdye@github.com>
Subject: t9210-scalar.sh fails with SANITIZE=undefined
Date: Thu, 22 Sep 2022 06:04:37 -0400	[thread overview]
Message-ID: <YywzNTzd72tox8Z+@coredump.intra.peff.net> (raw)

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.

-Peff

             reply	other threads:[~2022-09-22 10:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-22 10:04 Jeff King [this message]
2022-09-22 12:35 ` t9210-scalar.sh fails with SANITIZE=undefined Derrick Stolee
2022-09-22 19:12   ` Jeff King
2022-09-22 22:09     ` Victoria Dye
2022-09-22 22:27       ` Jeff King
2022-09-22 22:56         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YywzNTzd72tox8Z+@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=vdye@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).