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

From: ZheNing Hu <adlternative@gmail.com>
Date: May 11, 2021, 06:28 UTC
Message-ID: <CAOLTT8QR4Ga41ADKhgB4=C7VgGbJEe5G5HSbcjRb51H2yQVRPA@mail.gmail.com>
In-Reply-To: <xmqqcztyovf7.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> 于2021年5月11日周二 上午10:29写道：

Show 15 quoted lines

>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> >     Change from last version:
> >     Prove that the bug may appear when using %(color) atom. And add
> >     corresponding test for it.
>
> >      ++test_expect_success '%(color:#aa22ac) must success' '
>
> s/success/succeed/
>
> But more importantly, I am not sure how this is supposed to
> demonstrate existing breakage around the %(push).  Did you mean to
> use %(push) instead of %(refname) or something?
>

Ah, I don’t think this scene of damage is related to using %(push) or %(refname).

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.

Thanks. -- ZheNing Hu