From: Stefan Sperling <stsp@stsp.name>
Subject: Re: `got import` segfaults on macOS (got-portable 0.85, 0.86)
To: Carlo Cabrera <carlo.antonio.cabrera@gmail.com>
Cc: gameoftrees@openbsd.org
Date: Wed, 15 Mar 2023 13:47:20 +0100

On Wed, Mar 15, 2023 at 07:35:36PM +0800, Carlo Cabrera wrote:
> Hi all,
> 
> We've been having trouble updating our got package at Homebrew. (See attempts at
> [1], [2].)
> 
> The following smoke test causes `got` to segfault:
> 
>     export GOT_AUTHOR="Flan Hacker <flan_hacker@openbsd.org>"
>     gotadmin init repo.git
>     mkdir import-dir
>     touch import-dir/{haunted,house}
>     got import -m "Initial commit" -r repo.git import-dir
> 
> This has been happening since got 0.85. got 0.84 does not have this problem.
> 
> Based on output from `lldb`, I believe the problem is a null pointer
> dereference:
> 
>     ❯ lldb -- got import -m "initial commit" -r repo.git import-dir
>     (lldb) target create "got"
>     Current executable set to 'got' (x86_64).
>     (lldb) settings set -- target.run-args  "import" "-m" "initial commit" "-r" "repo.git" "import-dir"
>     (lldb) r
>     Process 29562 launched: '/usr/local/bin/got' (x86_64)
>     Process 29562 stopped
>     * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
>         frame #0: 0x00007ff80d07c6b2 libsystem_platform.dylib`_platform_strlen + 18
>     libsystem_platform.dylib`:
>     ->  0x7ff80d07c6b2 <+18>: pcmpeqb (%rdi), %xmm0
>         0x7ff80d07c6b6 <+22>: pmovmskb %xmm0, %esi
>         0x7ff80d07c6ba <+26>: andq   $0xf, %rcx
>         0x7ff80d07c6be <+30>: orq    $-0x1, %rax
>     Target 0: (got) stopped.
>     (lldb) bt
>     * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
>       * frame #0: 0x00007ff80d07c6b2 libsystem_platform.dylib`_platform_strlen + 18
>         frame #1: 0x00007ff80cf346c8 libsystem_c.dylib`strdup + 18
>         frame #2: 0x0000000100001f79 got`cmd_import(argc=6, argv=0x00007ff7bfefe398) at got.c:788:13 [opt]
>         frame #3: 0x0000000100001c36 got`main(argc=6, argv=0x00007ff7bfefe398) at got.c:260:11 [opt]
>         frame #4: 0x00000001000c952e dyld`start + 462
>     (lldb) up
>     frame #1: 0x00007ff80cf346c8 libsystem_c.dylib`strdup + 18
>     libsystem_c.dylib`strdup:
>     ->  0x7ff80cf346c8 <+18>: movq   %rax, %rbx
>         0x7ff80cf346cb <+21>: incq   %rbx
>         0x7ff80cf346ce <+24>: movq   %rbx, %rdi
>         0x7ff80cf346d1 <+27>: callq  0x7ff80cfb5856            ; symbol stub for: malloc
>     (lldb) up
>     warning: got was compiled with optimization - stepping may behave oddly; variables may not be available.
>     frame #2: 0x0000000100001f79 got`cmd_import(argc=6, argv=0x00007ff7bfefe398) at got.c:788:13 [opt]
>        785                                  goto done;
>        786                          break;
>        787                  case 'm':
>     -> 788                          logmsg = strdup(optarg);
>        789                          if (logmsg == NULL) {
>        790                                  error = got_error_from_errno("strdup");
>        791                                  goto done;
> 
> In particular, `optarg` seems to be a null pointer, but I haven't yet worked out
> why this is the case.
> 
> Any assistance here would be appreciated.
> 
> Best,
> Carlo
> 
> [1] https://github.com/Homebrew/homebrew-core/pull/125107
> [2] https://github.com/Homebrew/homebrew-core/pull/125651

FWIW, this problem cannot be reproduced on OpenBSD.
So it is either specific to -portable or to MacOS/Homebrew.
I wonder if there are incompatible implementations of getopt()?

Is the test suite passing? ("make tests" in the -portable source tree)
The test suite should have caught this problem since the very first test
for 'got import' runs essentially the same scenario. If you are not yet
running the test suite as part of the packaging process then that is a
gap in your process which should be fixed ASAP.

test_import_basic() {
	local testname=import_basic
	local testroot=`mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX"`

	gotadmin init $testroot/repo

	mkdir $testroot/tree
	make_test_tree $testroot/tree

	got import -m 'init' -r $testroot/repo $testroot/tree \
		> $testroot/stdout