On Wed, Oct 04, 2017 at 04:02:12AM -0400, Jeff King wrote: > On Wed, Oct 04, 2017 at 04:19:52PM +0900, Junio C Hamano wrote: > > > * jt/partial-clone-lazy-fetch (2017-10-02) 18 commits > [...] > > The merge of this topic into jch (at 766f92478b0) causes the test suite > to fail when compiled with ASan/UBSan. The simplest reproduction I could > come up with is: > > $ make SANITIZE=address,undefined BLK_SHA1=1 && > GIT_DIR=nope ./git shortlog </dev/null >/dev/null > repository.c:69:31: runtime error: index 1869098813 out of bounds for type 'git_hash_algo [1]' > > Note that the series is fine by itself, it's only the merge which fails. > Which implies to me it's some funny interaction with bc/hash-algo (which > introduces the hash_algo concept). But I didn't dig further. > > +cc brian and Jonathan. Actually, I take it back. Bisecting between "master" and "pu" turned up that commit (and I verified that it fails at that commit but not at its first parent). But if I go directly to the tip of bc/hash-algo, I can see the failure there. And it bisects to 67a9dfcc00 (hash-algo: integrate hash algorithm support with repo setup, 2017-08-21). So I think it's a read of uninitialized data, and it may come and go as various topics tickle the compiler differently. And that would make jt/partial-clone-lazy-fetch a red herring. This seems to make it go away (on top of 67a9dfcc00): diff --git a/setup.c b/setup.c index 289e24811c..310afe9736 100644 --- a/setup.c +++ b/setup.c @@ -1126,7 +1126,9 @@ const char *setup_git_directory_gently(int *nongit_ok) repo_set_gitdir(the_repository, gitdir); setup_git_env(); } - repo_set_hash_algo(the_repository, repo_fmt.hash_algo); + /* This is only valid if we actually found a repository */ + if (startup_info->have_repository) + repo_set_hash_algo(the_repository, repo_fmt.hash_algo); } strbuf_release(&dir); The problem is that we may not have a real repository at all, but we still hit this code path, which tries to setup repository properties, if there's a $GIT_DIR set. I actually suspect this repo_set_hash_algo() line should go into check_repository_format_gently(), where we set other globals (and which should eventually take a pointer to struct repository rather than touching the_repository). But I'm sufficiently convinced now that this is just a bug in the RFC-marked bc/hash-algo topic, and in no danger of graduating to master soon. -Peff
