Re: [PATCH v2] Cygwin: ssp: Add AArch64 implementation

On 08/06/2026 07:08, Chandru Kumaresan wrote:
> Thanks for the review!
>> did you consider writing something like (untested): [SW_BREAKPOINT_SIZE refactor]
> Yes, adopted in v2. I introduced SW_BREAKPOINT_SIZE as a macro (1 for x86/x86_64, 4
> for aarch64, #error otherwise), unified the PendingBreakpoints struct to use
> real_insn[SW_BREAKPOINT_SIZE] throughout, and renamed the variable to brk_insn on both
> architectures. This eliminates the per-arch #ifdef blocks in add_breakpoint and remove_breakpoint.
> 
>> Hmmm... this is probably just generically right, I guess.
>> (re: only setting running=0 on !dwFirstChance for aarch64)
>   On Windows/AArch64, the single-step mechanism (PSTATE.SS) triggers a first-chance STATUS_SINGLE_STEP
> exception for every instruction we step through. If we set running=0 on first-chance exceptions,
> the profiler would exit on the very first step. Limiting it to second-chance (i.e. unhandled) exceptions
> means we only stop on genuine faults, matching the intent of the original x86 code.

Hmmm... I think the x86 code is wrong in that, if a program normally 
handles and continues from an exception, under profiling we'll just stop 
on the exception.

But I think we can safely leave that until someone actually encounters 
the maybe-problem.

> 
> All other issues (whitespace regression, spurious fprintf indent, missing cmdline_copy comment) are fixed in v2 as well.

Thanks. Applied.

> ---
>   winsup/utils/ssp.c | 154 ++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 140 insertions(+), 14 deletions(-)
> 
[...]
> -
> -  run_program (argv[optind]);
> +  {
> +    /* CreateProcess (called below with lpApplicationName == NULL) is
> +       documented to modify the lpCommandLine buffer in place.  argv[optind]
> +       points into our own argv, so passing it directly lets CreateProcess
> +       scribble on it; this was observed on aarch64-cygwin as the command
> +       line coming back mangled (e.g. 'test_hello.exe' -> 'st_hello.exxee')
> +       on later use.  Pass a private writable copy instead.  It is not freed
> +       because run_program() stores it in dll_info[0].name, which is read
> +       later when printing the DLL-profile table.  */
> +    char *cmdline_copy = strdup (argv[optind]);
> +    if (!cmdline_copy)
> +      {
> +   fprintf (stderr, "Out of memory duplicating cmdline\n");
> +   exit (1);
> +      }
> +    run_program (cmdline_copy);
> +  }

Hmmm... from the explanation above, it seems like this is in the wrong 
place and should be inside run_program, around the call to CreateProcess?

Otherwise, the same pointer which is passed to CreateProcess and 
potentially has its contents mutated by that is also assigned to 
dll_info[0].name, leading to a potentially corrupted string appearing in 
the DLL-profile table.

If that supposition is correct, I'd appreciate it if you could come up 
with a follow-up patch to change that.