LSan curiosity in t1300

LSan curiosity in t1300

[Jeff King]

If I run:

  make SANITIZE=leak GIT_TEST_PASSING_SANITIZE_LEAK=true CFLAGS=-O0

locally, then all tests pass or are skipped. But if switch to -O2, then
t1300 starts failing!

The command that supposedly leaks is this:

  git --config-env=foo.flag= config --bool foo.flag

and the backtrace looks like:

  Direct leak of 9 byte(s) in 1 object(s) allocated from:
      #0 0x56195ae6dc62 in __interceptor_malloc (git+0x78c62) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
      #1 0x56195b0ad6b6 in do_xmalloc wrapper.c
      #2 0x56195b0ad7bd in xmemdupz (git+0x2b87bd) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
      #3 0x56195af6f8d0 in git_config_push_env (git+0x17a8d0) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
      #4 0x56195ae71689 in handle_options git.c
      #5 0x56195ae7060f in cmd_main (git+0x7b60f) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
      #6 0x56195af370c6 in main (git+0x1420c6) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
      #7 0x7f45b9029209 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
      #8 0x7f45b90292bb in __libc_start_main csu/../csu/libc-start.c:389:3
      #9 0x56195ae420c0 in _start (git+0x4d0c0) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)

But here's the weird part. The function looks like this:

  void git_config_push_env(const char *spec)
  {
          char *key;
          const char *env_name;
          const char *env_value;
  
          env_name = strrchr(spec, '=');
          if (!env_name)
                  die(_("invalid config format: %s"), spec);
          key = xmemdupz(spec, env_name - spec);
          env_name++;
          if (!*env_name)
                  die(_("missing environment variable name for configuration '%.*s'"),
                      (int)(env_name - spec - 1), spec);
  
          env_value = getenv(env_name);
          if (!env_value)
                  die(_("missing environment variable '%s' for configuration '%.*s'"),
                      env_name, (int)(env_name - spec - 1), spec);
  
          git_config_push_split_parameter(key, env_value);
          free(key);
  }

And it is complaining that we've leaked "key". But as you can see, we
always free it. The problem is that for this particular invocation we
die("missing environment variable name"), so of course we don't make it
to the free(). Normally this is OK, though. The "key" variable is still
on the stack, so the leak-checker should realize that it's still
reachable.

But if you run this in a debugger, you'll find that under -O2 the "key"
variable has been optimized out! So LSan is producing the wrong result
due to the optimization. It doesn't know that "key" is conceptually
still reachable.

Maybe this is a known problem, but it was certainly surprising to me.
I'm not sure if we should do anything about it or not. It doesn't seem
to trigger in CI, even though I don't see us taking any steps there to
use -O0 or similar. So we can perhaps ignore it for now, and this
message can serve as a warning. But if we think LSan isn't reliable
under higher optimizations, we could perhaps teach the Makefile to
prefer -O0 when it sees SANITIZE=leak.

-Peff

[PATCH] tests: opt "git --config-env" test out of SANITIZE=leak

[Ævar Arnfjörð Bjarmason]

In 676063016ad (leak tests: mark some config tests as passing with
SANITIZE=leak, 2021-10-31) t1300*.sh as a whole was marked as passing
with SANITIZE=leak, but as reported[1] certain compiler options and
optimization levels will re-arrange the generated code to have the
sanitizer fire.

This issue is reproducible with e.g. clang v12 with CFLAGS=-O2. The
issue is that -fsanitize=leak will combine with other optimization
options, and as we eliminate functions, variables etc. the
-fsanitize=leak criteria for considering something a leak might
effectively change.

This is arguably not a compiler issue or bug, as the very notion of a
variable being "in scope" as far as the leak checker is concerned is
fuzzy at best in the face of compiler optimizations.

A similar issue came up before related to the config.c code in
[2]. There the consensus seemed to be to avoid hacks in the C code to
work around these compiler edge cases. In this case however skipping
one test is easy enough. We can deal with these "!SANITIZE_LEAK"
issues later, when we have fewer overall leaks.

1. https://lore.kernel.org/git/Yxl91jfycCo7O7Pp@coredump.intra.peff.net/
2. https://lore.kernel.org/git/patch-v3-5.6-9a44204c4c9-20211022T175227Z-avarab@gmail.com/
---

On Thu, Sep 08 2022, Jeff King wrote:

> If I run:
>
>   make SANITIZE=leak GIT_TEST_PASSING_SANITIZE_LEAK=true CFLAGS=-O0
>
> locally, then all tests pass or are skipped. But if switch to -O2, then
> t1300 starts failing!
>
> The command that supposedly leaks is this:
>
>   git --config-env=foo.flag= config --bool foo.flag
>
> and the backtrace looks like:
>
>   Direct leak of 9 byte(s) in 1 object(s) allocated from:
>       #0 0x56195ae6dc62 in __interceptor_malloc (git+0x78c62) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
>       #1 0x56195b0ad6b6 in do_xmalloc wrapper.c
>       #2 0x56195b0ad7bd in xmemdupz (git+0x2b87bd) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
>       #3 0x56195af6f8d0 in git_config_push_env (git+0x17a8d0) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
>       #4 0x56195ae71689 in handle_options git.c
>       #5 0x56195ae7060f in cmd_main (git+0x7b60f) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
>       #6 0x56195af370c6 in main (git+0x1420c6) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
>       #7 0x7f45b9029209 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
>       #8 0x7f45b90292bb in __libc_start_main csu/../csu/libc-start.c:389:3
>       #9 0x56195ae420c0 in _start (git+0x4d0c0) (BuildId: a5cfeb484fd14b6120fa26ff364fa2313fd23168)
>
> But here's the weird part. The function looks like this:
>
>   void git_config_push_env(const char *spec)
>   {
>           char *key;
>           const char *env_name;
>           const char *env_value;
>   
>           env_name = strrchr(spec, '=');
>           if (!env_name)
>                   die(_("invalid config format: %s"), spec);
>           key = xmemdupz(spec, env_name - spec);
>           env_name++;
>           if (!*env_name)
>                   die(_("missing environment variable name for configuration '%.*s'"),
>                       (int)(env_name - spec - 1), spec);
>   
>           env_value = getenv(env_name);
>           if (!env_value)
>                   die(_("missing environment variable '%s' for configuration '%.*s'"),
>                       env_name, (int)(env_name - spec - 1), spec);
>   
>           git_config_push_split_parameter(key, env_value);
>           free(key);
>   }
>
> And it is complaining that we've leaked "key". But as you can see, we
> always free it. The problem is that for this particular invocation we
> die("missing environment variable name"), so of course we don't make it
> to the free(). Normally this is OK, though. The "key" variable is still
> on the stack, so the leak-checker should realize that it's still
> reachable.

[Re-arranged your E-Mail a bit for the purposes of the flow of this
reply]

> Maybe this is a known problem, but it was certainly surprising to me.

It is surprising, but it's a (at least to me) known "issue". See
e.g. https://lore.kernel.org/git/patch-v3-5.6-9a44204c4c9-20211022T175227Z-avarab@gmail.com/
for a previous discussion between us. I took the consensus there to be
that we shouldn't bend over backwards to worry about this.

I have been diligently ensuring that I don't mark tests as leak-free
if they "pass", but then "fail" on higher optimization levels.

But I see this one slipped through because I locally changed my
default compiler from "clang" to "gcc" (again) a while ago, and thus
missed testing clang at higher optimization levels. Usually it's GCC
that's unhappy about this sort of thing.

> But if you run this in a debugger, you'll find that under -O2 the "key"
> variable has been optimized out! So LSan is producing the wrong result
> due to the optimization. It doesn't know that "key" is conceptually
> still reachable.
> [...]

I don't know if we (or the compiler implementors) can call it the
"wrong" result. Just as some warnings only pop up depending on
optimization levels, this behavior also depends on it.

I've seen quite a few of these around die() codepaths. Presumably some
re-arranging/eliminaton of the code around that NORETURN codepath...

> [...]
> I'm not sure if we should do anything about it or not. It doesn't seem
> to trigger in CI, even though I don't see us taking any steps there to
> use -O0 or similar. So we can perhaps ignore it for now, and this
> message can serve as a warning. But if we think LSan isn't reliable
> under higher optimizations, we could perhaps teach the Makefile to
> prefer -O0 when it sees SANITIZE=leak.

I think it's good to not force -O0 with SANITIZE=leak, but this is
annoying, so let's do the below change. With it the tests pass for me
with clang under -O2/-O3.

FWIW if you play with the "check" mode you'll probably also find that
we have some tests that are "flaky" as far as the leak checking is
concerned ("t2012 t6415 t6435", I think, at least I skip those locally
when using it). IIRC some of that also depends on -On.

 t/t1300-config.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index c6661e61af5..a4953881db6 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1398,7 +1398,7 @@ test_expect_success 'git --config-env with missing value' '
 	grep "invalid config format: config" error
 '
 
-test_expect_success 'git --config-env fails with invalid parameters' '
+test_expect_success !SANITIZE_LEAK 'git --config-env fails with invalid parameters' '
 	test_must_fail git --config-env=foo.flag config --bool foo.flag 2>error &&
 	test_i18ngrep "invalid config format: foo.flag" error &&
 	test_must_fail git --config-env=foo.flag= config --bool foo.flag 2>error &&
-- 
2.37.3.1492.gbb5af15b83b

Re: LSan curiosity in t1300

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> And it is complaining that we've leaked "key". But as you can see, we
> always free it. The problem is that for this particular invocation we
> die("missing environment variable name"), so of course we don't make it
> to the free(). Normally this is OK, though. The "key" variable is still
> on the stack, so the leak-checker should realize that it's still
> reachable.
>
> But if you run this in a debugger, you'll find that under -O2 the "key"
> variable has been optimized out! So LSan is producing the wrong result
> due to the optimization. It doesn't know that "key" is conceptually
> still reachable.

Yikes.  

Is that related to the cause of the other patch from you, i.e.
wanting to get "registers" but unable to?  If they manage to get the
value of the register that holds 'key', would this disappear, I
wonder?

> Maybe this is a known problem, but it was certainly surprising to me.
> I'm not sure if we should do anything about it or not. It doesn't seem
> to trigger in CI, even though I don't see us taking any steps there to
> use -O0 or similar. So we can perhaps ignore it for now, and this
> message can serve as a warning. But if we think LSan isn't reliable
> under higher optimizations, we could perhaps teach the Makefile to
> prefer -O0 when it sees SANITIZE=leak.

Nice note to leave here, I agree.

Re: LSan curiosity in t1300

[Jeff King]

On Thu, Sep 08, 2022 at 11:03:05AM -0700, Junio C Hamano wrote:

> > But if you run this in a debugger, you'll find that under -O2 the "key"
> > variable has been optimized out! So LSan is producing the wrong result
> > due to the optimization. It doesn't know that "key" is conceptually
> > still reachable.
> 
> Yikes.  
> 
> Is that related to the cause of the other patch from you, i.e.
> wanting to get "registers" but unable to?  If they manage to get the
> value of the register that holds 'key', would this disappear, I
> wonder?

I don't think so. t1300 fails consistently for me with -O2, and it
doesn't have the "register" message (and never came up as one of the
racy losers). I think the "register" thing is mostly just a harmless
warning that doesn't change the output, but simply confuses our "did
lsan produce any logs" check.

-Peff

Re: [PATCH] tests: opt "git --config-env" test out of SANITIZE=leak

[Jeff King]

On Thu, Sep 08, 2022 at 05:42:54PM +0200, Ævar Arnfjörð Bjarmason wrote:

> This is arguably not a compiler issue or bug, as the very notion of a
> variable being "in scope" as far as the leak checker is concerned is
> fuzzy at best in the face of compiler optimizations.
> 
> A similar issue came up before related to the config.c code in
> [2]. There the consensus seemed to be to avoid hacks in the C code to
> work around these compiler edge cases. In this case however skipping
> one test is easy enough. We can deal with these "!SANITIZE_LEAK"
> issues later, when we have fewer overall leaks.

IMHO this is the wrong approach. It is playing whack-a-mole with
individual false positives, when we know the root cause is compiler
optimizations. We should be invoking the compiler in a way that gives us
the most reliable result.

In the long run, when all leaks are plugged, we'd want to ditch the
whole SANITIZE_LEAK system anyway. So we are better off preventing false
positives than trying to gloss over them.

> > Maybe this is a known problem, but it was certainly surprising to me.
> 
> It is surprising, but it's a (at least to me) known "issue". See
> e.g. https://lore.kernel.org/git/patch-v3-5.6-9a44204c4c9-20211022T175227Z-avarab@gmail.com/
> for a previous discussion between us. I took the consensus there to be
> that we shouldn't bend over backwards to worry about this.

Thanks, I'd forgotten about that conversation. I stand by my comments
there that we shouldn't be changing the code. But I really think just
using -O0 for the leak-check is a better solution than trying to label
false positives.

> But I see this one slipped through because I locally changed my
> default compiler from "clang" to "gcc" (again) a while ago, and thus
> missed testing clang at higher optimization levels. Usually it's GCC
> that's unhappy about this sort of thing.

FWIW, I found this using clang-14, because the gcc on my system (12.2.0)
does not seem to find actual leaks. It couldn't see any of the ones in
t1060 that I fixed yesterday.

> > But if you run this in a debugger, you'll find that under -O2 the "key"
> > variable has been optimized out! So LSan is producing the wrong result
> > due to the optimization. It doesn't know that "key" is conceptually
> > still reachable.
> > [...]
> 
> I don't know if we (or the compiler implementors) can call it the
> "wrong" result. Just as some warnings only pop up depending on
> optimization levels, this behavior also depends on it.

I think it's different than the case of warnings. In those cases the
optimizations let the compiler see more of what's going on in the code,
which lets them find a warn-able offense that they wouldn't otherwise
see (e.g., inlining a function lets the compiler see an invariant).
Those are real problems in the code that -O0 simply doesn't catch.

But this is different: there is no problem in the code. It is just that
the optimization of removing the variable does not work well with how
the leak-checker works. There is nothing we can do in the code to fix
it; the C conceptual model of variable scoping is all we have to work
with, and according to that, there is no leak. C's "as if" rule means
that the compiler can change the program as long as the behavior is the
same. And it is, from the perspective of the rest of the program. The
issue is that the leak-checker is itself a sort of undefined behavior;
it expects to poke at random parts of the stack at any moment and find a
consistent state, which is not something a normal C program can do.

> I've seen quite a few of these around die() codepaths. Presumably some
> re-arranging/eliminaton of the code around that NORETURN codepath...

The compiler probably does this kind of re-arranging and elimination all
the time. The reason it comes up around die() is that it means we invoke
the leak-checker mid-function. Even though the "key" variable here was
eliminated, the compiler would need to arrange that the pointer value is
_somewhere_ so that when we hit the end of the function we can call that
free(). But when we exit in the middle, we end up doing the leak-check
in a funny state.

It may be that this is influenced by the NORETURN flag. Presumably "key"
is held in a register, and the call to die(), knowing that it does not
need to return, may choose not to preserve that register. So turning
NORETURN into a noop _might_ help, but only because it's giving the
optimizer less information to work with (and it might not if the
optimizer can figure it out on its own via inline functions, for
example). The root cause is the optimizer, so disabling it is the
most complete fix.

-Peff

Re: [PATCH] tests: opt "git --config-env" test out of SANITIZE=leak

[Ævar Arnfjörð Bjarmason]

On Thu, Sep 08 2022, Jeff King wrote:

> On Thu, Sep 08, 2022 at 05:42:54PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> This is arguably not a compiler issue or bug, as the very notion of a
>> variable being "in scope" as far as the leak checker is concerned is
>> fuzzy at best in the face of compiler optimizations.
>> 
>> A similar issue came up before related to the config.c code in
>> [2]. There the consensus seemed to be to avoid hacks in the C code to
>> work around these compiler edge cases. In this case however skipping
>> one test is easy enough. We can deal with these "!SANITIZE_LEAK"
>> issues later, when we have fewer overall leaks.
>
> IMHO this is the wrong approach. It is playing whack-a-mole with
> individual false positives, when we know the root cause is compiler
> optimizations. We should be invoking the compiler in a way that gives us
> the most reliable result.
>
> In the long run, when all leaks are plugged, we'd want to ditch the
> whole SANITIZE_LEAK system anyway. So we are better off preventing false
> positives than trying to gloss over them.

In the long run when all leaks are plugged I'd prefer to be able to
compile a git with CFLAGS=-O3 and -fsanitize=leak, and have it run "git
config" without erroring out about a leak.

So I'd really prefer to keep this patch as-is. I'd agree with you if the
"whack-a-mole" was painful, but in this case it's really not. I think
it's just this one edge-case in the whole codebase (out of the things
marked leak-free).

That's inching us closer to what I think should be the end goal,
i.e. this SANITIZE=leak is just a transition mechanism, so having it
enforce -O0 would mean that we can't have -fsanitize=leak Just Work in
the future (both for the tests & when manually compiled).

>> I don't know if we (or the compiler implementors) can call it the
>> "wrong" result. Just as some warnings only pop up depending on
>> optimization levels, this behavior also depends on it.
>
> I think it's different than the case of warnings. In those cases the
> optimizations let the compiler see more of what's going on in the code,
> which lets them find a warn-able offense that they wouldn't otherwise
> see (e.g., inlining a function lets the compiler see an invariant).
> Those are real problems in the code that -O0 simply doesn't catch.
>
> But this is different: there is no problem in the code. It is just that
> the optimization of removing the variable does not work well with how
> the leak-checker works. There is nothing we can do in the code to fix
> it; the C conceptual model of variable scoping is all we have to work
> with, and according to that, there is no leak. C's "as if" rule means
> that the compiler can change the program as long as the behavior is the
> same. And it is, from the perspective of the rest of the program. The
> issue is that the leak-checker is itself a sort of undefined behavior;
> it expects to poke at random parts of the stack at any moment and find a
> consistent state, which is not something a normal C program can do.

This is just from vague recollection, and I couldn't find an example now
with some light digging (which was just seeing if any of the tests with
a "!SANITIZE_LEAK" prereq passed on -O0, but not -O3 with clang), but I
think I've run into cases where higher optimization levels have also
spotted genuine leaks that I've fixed.

Re: [PATCH] tests: opt "git --config-env" test out of SANITIZE=leak

[Jeff King]

On Fri, Sep 23, 2022 at 10:28:29AM +0200, Ævar Arnfjörð Bjarmason wrote:

> > In the long run, when all leaks are plugged, we'd want to ditch the
> > whole SANITIZE_LEAK system anyway. So we are better off preventing false
> > positives than trying to gloss over them.
> 
> In the long run when all leaks are plugged I'd prefer to be able to
> compile a git with CFLAGS=-O3 and -fsanitize=leak, and have it run "git
> config" without erroring out about a leak.

Why? Do you want to run a leak-checking copy of Git all the time? If so,
I have bad news for you performance-wise. Running the tests marked as
leak-passing with -O2 (but not -fsanitize=leak) takes ~101s of CPU.
Running with -O0 takes ~111s. Running with -fsanitize=leak takes ~241s.
So the improvement from compiler optimizations is not a big win there,
relatively speaking.

Or are you thinking that -O3 reveals new information we would not find
under other optimization levels? I don't think this is the case. While
that does sometimes find new opportunities for static analysis (via
inlining code, etc), I don't think it helps with run-time analysis. And
as we've seen here, it actively makes things _worse_ by introducing
false positives.

> So I'd really prefer to keep this patch as-is. I'd agree with you if the
> "whack-a-mole" was painful, but in this case it's really not. I think
> it's just this one edge-case in the whole codebase (out of the things
> marked leak-free).

Is it just this one spot? This is already the second one we've discussed
on the list, and I think you indicated there were more spots where you
intentionally held back on setting TEST_PASSES_SANITIZE_LEAK when you
saw hits under higher optimization levels.

It really is a potential problem anywhere we'd call a NORETURN function,
because the compiler (rightly) realizes there is no point in making sure
we can call a later free() that we'll never reach.

> That's inching us closer to what I think should be the end goal,
> i.e. this SANITIZE=leak is just a transition mechanism, so having it
> enforce -O0 would mean that we can't have -fsanitize=leak Just Work in
> the future (both for the tests & when manually compiled).

I think TEST_PASSES_SANITIZE_LEAK is a transition mechanism. But we'll
always want SANITIZE=leak, and I don't think it makes sense for ordinary
builds to use it.

> This is just from vague recollection, and I couldn't find an example now
> with some light digging (which was just seeing if any of the tests with
> a "!SANITIZE_LEAK" prereq passed on -O0, but not -O3 with clang), but I
> think I've run into cases where higher optimization levels have also
> spotted genuine leaks that I've fixed.

If you have a counter-example, I'm all ears, but I have trouble
imagining how -O3 could actually find a leak not present in -O0. Again,
because we're testing runtime behavior, and the optimizations should not
be changing the visible behavior of our program (and the problem is the
leak checker itself is testing "invisible" things that may not actually
be important, like the state of a variable on the stack when we call
exit()).

-Peff

Re: [PATCH] tests: opt "git --config-env" test out of SANITIZE=leak

[Ævar Arnfjörð Bjarmason]

On Fri, Sep 23 2022, Jeff King wrote:

> On Fri, Sep 23, 2022 at 10:28:29AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> > In the long run, when all leaks are plugged, we'd want to ditch the
>> > whole SANITIZE_LEAK system anyway. So we are better off preventing false
>> > positives than trying to gloss over them.
>> 
>> In the long run when all leaks are plugged I'd prefer to be able to
>> compile a git with CFLAGS=-O3 and -fsanitize=leak, and have it run "git
>> config" without erroring out about a leak.
>
> Why? Do you want to run a leak-checking copy of Git all the time? 

Not all the time, yes, and I've been getting closer to compiling my
daily driver with it..

> If so,
> I have bad news for you performance-wise. Running the tests marked as
> leak-passing with -O2 (but not -fsanitize=leak) takes ~101s of CPU.
> Running with -O0 takes ~111s. Running with -fsanitize=leak takes ~241s.
> So the improvement from compiler optimizations is not a big win there,
> relatively speaking.

Yeah, I know it sucks, but I use it for interactive use, "git push" and
the like, so I usually don't care if it's ~2x slower. I even run with
SANITIZE=address sometimes.

Wanting to have non-O0 there is less about thinking the higher -On
helps, and more to avoid it being a special snowflake, i.e. if I run
with -O2 -g usually I'd like to just add -fsanitize=leak to that, and
not have to also change the optimization level.

> Or are you thinking that -O3 reveals new information we would not find
> under other optimization levels? I don't think this is the case. While
> that does sometimes find new opportunities for static analysis (via
> inlining code, etc), I don't think it helps with run-time analysis. And
> as we've seen here, it actively makes things _worse_ by introducing
> false positives.

I think this (and to your "conter-example" below) is correct on your
part. I.e. I don't see a good reason for why this would happen.

I have been able to reliably reproduce some leaks as being flaky (and
have avoided adding them to the tests). I wonder if that explains it,
i.e. there was another underlying issue, and the optimization level
happened to trigger some race (or whatever was going on there, I haven't
looked in any depth into those either...).

>> So I'd really prefer to keep this patch as-is. I'd agree with you if the
>> "whack-a-mole" was painful, but in this case it's really not. I think
>> it's just this one edge-case in the whole codebase (out of the things
>> marked leak-free).
>
> Is it just this one spot? This is already the second one we've discussed
> on the list, and I think you indicated there were more spots where you
> intentionally held back on setting TEST_PASSES_SANITIZE_LEAK when you
> saw hits under higher optimization levels.

Probably not the only spot, but the only spot under the
TEST_PASSES_SANITIZE_LEAK umbrella.

> It really is a potential problem anywhere we'd call a NORETURN function,
> because the compiler (rightly) realizes there is no point in making sure
> we can call a later free() that we'll never reach.

FWIW I'm not sure it's NORETURN, and haven't had time to dig...