Re: [PATCH v4] [GSOC] ref-filter: fix read invalid union member bug

From: Junio C Hamano <gitster@pobox.com>
Date: May 11, 2021, 09:30 UTC
Message-ID: <xmqqlf8lmxca.fsf@gitster.g>
In-Reply-To: <CAOLTT8QR4Ga41ADKhgB4=C7VgGbJEe5G5HSbcjRb51H2yQVRPA@mail.gmail.com>

ZheNing Hu <adlternative@gmail.com> writes:

Show 36 quoted lines

> We are just in the process of using `populate_value()`, if the atom we
> specify meets the following conditions, then the condition of
> atom->u.remote_ref.push!=0 will be established.
>
> 1. The atom that triggers the bug , its "if" condition order must after
> "if (atom->u.remote_ref.push)", such as %(refname) or %(worktreepath),
> they can be executed correctly because their order is before "push".
>
> 2. The member size in used_atom.u corresponding to the atom must
> larger than 17 bytes, because the offset of "u.remote_ref.push" in
> "u.remote_ref" is 17, the satisfied atoms are only "%(color)" and
>  "%(contents)", their corresponding members are u.color and u.contents.
>
> 3. We happen to be able to fill in the 17th position of these structures,
>  which makes atom->u.remote_ref.push not equal to 0 established.
>
> So this kind of bug is not related to %(push), an atom that satisfies
> the above conditions will make `if (atom->u.remote_ref.push)` be true.
> then execute the program logic related to `%(push)`.
>
> Now, we only have `%(color)` can trigger it "sometime",
> It is unpredictable to fill in the 17th byte of used_atom.u.color,
> so we cannot track all the atoms related to this bug.
>
> git for-each-ref --format="%(color:#aa22ac)%(refname)"
> git for-each-ref --format="%(color:#aa22ad)%(refname)"
>
> will trigger the bug.
>
> git for-each-ref --format="%(color:#aa22ae)%(refname)"
> git for-each-ref --format="%(color:#aa22af)%(refname)"
>
> will not trigger the bug.
>
> In other words, we cannot use a perfect test set to cover all broken.
> So now `%(color:#aa22ac)` is enough for explain the problem of this bug.

Well, the thing is,

    $ git checkout 49f38e2d ;# (The fifteenth batch, 2021-05-10)
    $ git am -s mbox
    $ git show --stat --oneline
    39509d100a (HEAD) ref-filter: fix read invalid union member bug
     ref-filter.c                   |  2 +-
     t/t6302-for-each-ref-filter.sh | 18 ++++++++++++++++++
     2 files changed, 19 insertions(+), 1 deletion(-)
    $ git show ref-filter.c | git apply -R ;# revert only the fix
    $ make -j32 && make -C t T=t6302-*.sh

does not seem to break anything. Perhaps there is something more than the "17th byte" thing (like structure padding that may vary depending on the compiler and architecture)?