Segmentation fault on cherry-pick #952
Comments
|
I now rebased the change on Gerrit directly (now set 5) and all of a sudden the patch could be applied again. I'll still keep this report open, since no matter what happened here, a segmentation fault must never happen. |
|
I seriously hope you kept a |
|
No, I did not, but running the above cherry-pick does still segfault. ;-) Shall I upload the file here, or should I send it somewhere? > 200MB |
|
@liayn thanks for keeping the |
|
@dscho I'm on the road a bit, so "streaming" the file to you is a bit hard. I uploaded it to our webserver now. Can I send you the link in a PM somehow? |
@liayn sure. I'm on https://gitter.im/git-for-windows/git, for example. Also, all of my commits are signed off using my email address. |
|
For the record, I received the rather large |
|
For the record: I did manage to respond to your mails earlier. Did so now. Let's see if we can still manage to see what's been happening. Thanks so far! |
|
@liayn would it be possible to install the Git for Windows SDK and try to run the cherry-pick through |
|
Sure, will give it a shot tomorrow. |
|
Okay, I followed https://github.com/git-for-windows/git/wiki/Debugging-Git and kicked the optimization. I skipped step 3, since this seems to be governed by a CFLAGS check already. @dscho Any suggestion for me? |
|
Okay... one should update the wiki and should mention that I should not use the build is running now |
|
here we go: guess you need some more info, right? |
I did that now. For future record: please just edit the wiki next time you see something wrong there. It's a wiki for that very reason: so that you (and everybody else) do not need to wait for me to do everything.
Yes. You may see different threads using the If that does not help shed light into the issue, it would be best to single-step, or a variation thereof. This is how I would go about it:
This will need a little bit of tenacity, but should get you consistently to the culprit. |
Sorry, wasn't aware that this wiki is open for everyone. I'm more used to those "closed" wikis, where only members can edit.
Okay, will do a binary search. Lets see. Thanks for the detailed info. I'm so much used to GUI based debuggers nowadays. ;-) |
Looks like threads 2 to 4 are in waiting state for work. |
|
Bad news... (I ran the SDK as admin) Not sure how to proceed here. |
That means that the file was compiled with ASLR.
This is not the |
|
Oh dear... stupidity will never be exterminated. Here we go: I guess |
|
The trace: |
read-cache.c: This is where the NULL comes from that causes the segfault later on. The |
|
Okay, that's better, and it points to a terrible thing, too: The static int add_cacheinfo(struct merge_options *o,
unsigned int mode, const struct object_id *oid,
const char *path, int stage, int refresh, int options)
{
struct cache_entry *ce;
int ret;
ce = make_cache_entry(mode, oid ? oid->hash : null_sha1, path, stage, 0);
if (!ce)
return err(o, _("addinfo_cache failed for path '%s'"), path);
ret = add_cache_entry(ce, options);
if (refresh) {
struct cache_entry *nce;
nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
if (nce != ce)
ret = add_cache_entry(nce, options);
}
return ret;
}Line 239 is that last So it seems that something makes the return value of The fact that The first one triggers when an early part of the file turned into a symbolic link (in which case the entry is no longer valid, as we would track the symbolic link, not the target). The second one triggers when the file does not even exist. And the third one triggers when the entry was modified (or at least, is marked as modified, which may be the case in your setup if the line ending settings somehow contradict each other). It would be really interesting to find out what happened there. Could you find out what happened with In any case, I think what should happen here is that the cherry-pick should complain about an index entry that is not up-to-date, something like this: diff --git a/merge-recursive.c b/merge-recursive.c
index 9041c2f149..609061f58a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -235,6 +235,8 @@ static int add_cacheinfo(struct merge_options *o,
struct cache_entry *nce;
nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
+ if (!nce)
+ return err(o, _("addinfo: '%s' is not up-to-date"), path);
if (nce != ce)
ret = add_cache_entry(nce, options);
}Could you test that? |
Ah, it looks more and more as if this is a line ending problem. |
|
As written above it is clearly a bug that In general I always install git for windows with the setting checkout-as-is. I simply hate this weird auto-conversion of line endings. Put stuff on my disk as is and the editor will cope with the rest.
|
|
Adding the NULL-check to the source now results into: We are on a very good track. ;-) |
|
So the worktree file says CR/LF, the index says LF. I guess that is why |
Just to make sure, |
|
It does not matter, apparently. I modified that For the record, this is not a minimal example. I think I boiled it down to this recipe: git init
echo first >first && git add first && git commit -m first
git checkout -b branch
echo unrelated >unrelated & git add unrelated && git commit -m unrelated
git checkout @{-1}
git mv first renamed && git commit -m renamed
echo modified >renamed
git cherry-pick branchLet's try to turn that into a patch to the test suite. |
|
For the record: |
|
@liayn yeah, line endings. You asked for the line endings to be identical to the ones in the repository, but somehow got CR/LFs into your working file. The bug is reproducible with a regular modified file, though. |
|
Maybe to be ultra-precise: Don't know if it is good or bad that you can reproduce this with regular files. |
|
It's good. 😁 |
In git-for-windows#952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Under very particular circumstances, merge-recursive's `add_cacheinfo()` function gets a `NULL` returned from `refresh_cache_entry()` without expecting it, and subsequently passes it to `add_cache_entry()` which consequently crashes. Let's not crash. This fixes git-for-windows#952 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In git-for-windows#952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Under very particular circumstances, merge-recursive's `add_cacheinfo()` function gets a `NULL` returned from `refresh_cache_entry()` without expecting it, and subsequently passes it to `add_cache_entry()` which consequently crashes. Let's not crash. This fixes git-for-windows#952 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In git-for-windows#952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Under very particular circumstances, merge-recursive's `add_cacheinfo()` function gets a `NULL` returned from `refresh_cache_entry()` without expecting it, and subsequently passes it to `add_cache_entry()` which consequently crashes. Let's not crash. This fixes git-for-windows#952 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In git-for-windows#952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1335d76 ("merge: avoid "safer crlf" during recording of merge results", 2016-07-08) tried to split make_cache_entry() call made with CE_MATCH_REFRESH into a call to make_cache_entry() without one, followed by a call to add_cache_entry(), refresh_cache() and another add_cache_entry() as needed. However the conversion was botched in that it forgot that refresh_cache() can return NULL, which was handled correctly in make_cache_entry() but in the updated code. This fixes git-for-windows#952 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In git-for-windows#952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
A certain scenario that could cause a crash in cherry-pick [no longer causes that](git-for-windows/git#952). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Under very particular circumstances, merge-recursive's `add_cacheinfo()` function gets a `NULL` returned from `refresh_cache_entry()` without expecting it, and subsequently passes it to `add_cache_entry()` which consequently crashes. Let's not crash. This fixes #952 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In #952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Under very particular circumstances, merge-recursive's `add_cacheinfo()` function gets a `NULL` returned from `refresh_cache_entry()` without expecting it, and subsequently passes it to `add_cache_entry()` which consequently crashes. Let's not crash. This fixes #952 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In #952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Under very particular circumstances, merge-recursive's `add_cacheinfo()` function gets a `NULL` returned from `refresh_cache_entry()` without expecting it, and subsequently passes it to `add_cache_entry()` which consequently crashes. Let's not crash. This fixes #952 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In git-for-windows/git#952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> (cherry picked from commit 05f2dfb965476a59050b7c3446b1281bdcac7051) Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Gbp-Pq: Name cherry-pick-demonstrate-a-segmentation-fault.diff
1335d76e45 ("merge: avoid "safer crlf" during recording of merge
results", 2016-07-08) tried to split make_cache_entry() call made
with CE_MATCH_REFRESH into a call to make_cache_entry() without one,
followed by a call to add_cache_entry(), refresh_cache() and another
add_cache_entry() as needed. However the conversion was botched in
that it forgot that refresh_cache() can return NULL, which was
handled correctly in make_cache_entry() but in the updated code.
This fixes git-for-windows/git#952
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
(cherry picked from commit 55e9f0e5c9a918c246b7eae1fe2a2e954f6426af)
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Gbp-Pq: Name merge-recursive-handle-NULL-in-add_cacheinfo-correctl.diff
In git-for-windows/git#952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> (cherry picked from commit 05f2dfb965476a59050b7c3446b1281bdcac7051) Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Gbp-Pq: Name cherry-pick-demonstrate-a-segmentation-fault.diff
1335d76e45 ("merge: avoid "safer crlf" during recording of merge
results", 2016-07-08) tried to split make_cache_entry() call made
with CE_MATCH_REFRESH into a call to make_cache_entry() without one,
followed by a call to add_cache_entry(), refresh_cache() and another
add_cache_entry() as needed. However the conversion was botched in
that it forgot that refresh_cache() can return NULL, which was
handled correctly in make_cache_entry() but in the updated code.
This fixes git-for-windows/git#952
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
(cherry picked from commit 55e9f0e5c9a918c246b7eae1fe2a2e954f6426af)
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Gbp-Pq: Name merge-recursive-handle-NULL-in-add_cacheinfo-correctl.diff
Setup
defaults?
The repository is the TYPO3 CMS repository git://git.typo3.org/Packages/TYPO3.CMS.git
I run multiple worktrees for master/7-6/6-2/.
Details
Bash
Minimal, Complete, and Verifiable example
this will help us understand the issue.
The commit fetched from Gerrit should be applied on top.
Segmentation faultURL to that repository to help us with testing?
A colleague tried the same command and it works.
I tried creating another "master" branch, but cherry-picking segfaults as well.
Cherry-picking onto another branch (different worktree) works as expected.
I run
git gcgit fsck ...git prune...and so on, nothing helped.I did a complete new clone of the repo and applying the patch did work, but I actually do not want to redo my whole setup on a new checkout and I want to help finding the reason for a segfault.
Any help really appreciated. I could also provide my whole local repository for testing purposes, nothing confidential involved.
The text was updated successfully, but these errors were encountered: