git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: ZheNing Hu <adlternative@gmail.com>
Cc: ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com>,
	Git List <git@vger.kernel.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v4] [GSOC] ref-filter: fix read invalid union member bug
Date: Tue, 11 May 2021 18:30:29 +0900	[thread overview]
Message-ID: <xmqqlf8lmxca.fsf@gitster.g> (raw)
In-Reply-To: <CAOLTT8QR4Ga41ADKhgB4=C7VgGbJEe5G5HSbcjRb51H2yQVRPA@mail.gmail.com> (ZheNing Hu's message of "Tue, 11 May 2021 14:28:32 +0800")

ZheNing Hu <adlternative@gmail.com> writes:

> 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)?

  reply	other threads:[~2021-05-11  9:30 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-05 15:31 [PATCH] [GSOC] ref-filter: solve bugs caused by enumeration ZheNing Hu via GitGitGadget
2021-05-06  1:53 ` Junio C Hamano
2021-05-06  5:02   ` ZheNing Hu
2021-05-06  5:35     ` Junio C Hamano
2021-05-06 10:39       ` ZheNing Hu
2021-05-06 11:20         ` Junio C Hamano
2021-05-06 11:52           ` ZheNing Hu
2021-05-06 21:20             ` Junio C Hamano
2021-05-07  4:32               ` ZheNing Hu
2021-05-07  4:49                 ` Junio C Hamano
2021-05-07  5:09                   ` ZheNing Hu
2021-05-06 16:31 ` [PATCH v2] [GSOC] ref-filter: fix read invalid union member bug ZheNing Hu via GitGitGadget
2021-05-08 15:26   ` [PATCH v3] " ZheNing Hu via GitGitGadget
2021-05-10  7:21     ` Junio C Hamano
2021-05-10 12:35       ` ZheNing Hu
2021-05-10  7:27     ` Junio C Hamano
2021-05-10 12:51       ` ZheNing Hu
2021-05-10 15:01     ` [PATCH v4] " ZheNing Hu via GitGitGadget
2021-05-11  2:29       ` Junio C Hamano
2021-05-11  6:28         ` ZheNing Hu
2021-05-11  9:30           ` Junio C Hamano [this message]
2021-05-11 11:47             ` ZheNing Hu
2021-05-11 13:12               ` Junio C Hamano
2021-05-11 13:31                 ` ZheNing Hu
2021-05-11 15:35       ` [PATCH v5] " ZheNing Hu via GitGitGadget
2021-05-12  1:36         ` Junio C Hamano
2021-05-12 10:37           ` ZheNing Hu
2021-05-12 12:12         ` [PATCH v6] " ZheNing Hu via GitGitGadget
2021-05-12 23:24           ` Junio C Hamano
2021-05-13  9:29             ` ZheNing Hu
2021-05-13 15:13           ` [PATCH v7] " ZheNing Hu via GitGitGadget

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

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

  git send-email \
    --in-reply-to=xmqqlf8lmxca.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=adlternative@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.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.
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 NNTP newsgroup(s).