On Tue, 4 Jan 2022 at 13:21, Jonathan Wakely <cxx@kayari.org> wrote:


On Mon, 3 Jan 2022, 14:02 René Ferdinand Rivera Morell via SG15, <sg15@lists.isocpp.org> wrote:
On Mon, Jan 3, 2022 at 12:22 AM Daniela Engert via SG15 <sg15@lists.isocpp.org> 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.

Both GCC and Clang treat __builtin_trap() as noreturn:
https://llvm.org/docs/LangRef.html#llvm-trap-intrinsic
So it can't be used for a breakpoint.

 
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 ...".