On Mon, 3 Jan 2022, 14:02 René Ferdinand Rivera Morell via SG15, <
sg15_at_[hidden]> wrote:

> On Mon, Jan 3, 2022 at 12:22 AM Daniela Engert via SG15 <
> sg15_at_[hidden]> wrote:
>
>> Am 03.01.2022 um 04:30 schrieb René Ferdinand Rivera Morell via SG15:
>>
>> I just posted for review two tooling related proposals:
>>
>> * https://wg21.link/P2514R0 -- std::breakpoint
>> * https://wg21.link/P2514R0 -- std::is_debugger_present
>>
>> René, was this meant to be the same link?
>>
>
> LOL, indeed.. It should be:
>
> * https://wg21.link/P2514R0 -- std::breakpoint
>

The wording needs work. "*Remarks*: When this function is executed," should
be *Effects*.

It looks like it's talking about some normative property of the execution
environment, but there is no normative meaning of "running under a
debugger" or "handed back to the program"

I don't think mentioning GCC's __builtin_trap() as implementation
experience is useful. I'd say it's actively harmful. __builtin_trap() is
not a breakpoint. The docs say "This function causes the program to exit
abnormally." On some platforms (including x86) it raises a SIGILL signal
and ignoring it is undefined behaviour. I think the correct implementation
is to use raise(SIGTRAP) (or platform-specific assembly like asm("int3") on
x86, which is what Clang's __builtin_debugtrap() does). I checked what
Izzy's implementation did, and it just doesn't support Linux or GCC *at all*.
It certainly doesn't use __builtin_trap().. It looks like the only
implementation experience for linux is Unreal Engine 4, assuming that does
actually work for linux. https://github.com/scottt/debugbreak is a much
better reference implementation, although still wrong (it uses
defined(__APPLE__) where it should use defined(__clang__) which means it
doesn't work with GCC on aarch64).




> * https://wg21.link/P2515R0 -- std::is_debugger_present
>

For the second paper:

bool is_debugger_present() noexcept;

*Returns*: If the program is currently running under a debugger it returns
true, otherwise false.

*Remarks*: If not supported by the implementation it returns false.
This wording needs work too. Again, there is no normative meaning of
"running under a debugger", and no indication of when that is or isn't
supported by the implementation. The description of the effects (and the
name of the function!) is a bit Windows-specific. The suggested
implementation strategy for other operating systems actually seems to refer
to the ptrace system call. I think that means it will return true when the
program is being run under strace or gcov, because ptrace is used for more
than just debuggers.

I think it would be better to make it a bit more handwavy, and refer to a
debugger in a note. Maybe something that refers to the program's execution
environment being traced by another program. Then clarify in a note that it
is typically true when a debugger is attached to the program. We probably
also want the magic words "implementation-defined" to appear in the
normative wording, so that implementations have to document what it does,
so POSIX systems can make it clear that it's true for more cases than just
when a debugger is attached.

I would expect more co-operation between these papers. They don't even
mention each other, but presumably the "implementation-defined check" that
breakpoint() does should be is_debugger_present(), no? They should
cross-reference each other, and maybe say something like "if Pnnnn is
accepted this should be ...".