Thank you Jonathan for the feedback. I'm making changes per your suggestions.

On Tue, Jan 4, 2022 at 7:22 AM 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 looked back at LEWG discussion of Izzy's predecessor paper and indeed do see that as being the suggested route for this.

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.

From personal experience I can say the UE4 implementation does work on 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).

Thank you so much for that implementation reference. Indeed my implementation attempts to do the equivalent of SIGTRAP on all platforms. Of which the intricacies are thankfully taken care of by using <https://github.com/nemequ/portable-snippets/tree/master/debug-trap>. But it does fallback on __builtin_trap as a last resort in one case, and a SIGABRT in another.

I think I need to add psnip_trap as implementation experience also.

* 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 agree. Although I wish we acknowledged the tools around C++ more in the standard it makes sense to keep such things in non-normative notes until a defined tooling future exists.

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

I also debated on merging the two papers together as an option. What do you think? Is is better to keep them apart (theory being that it's easier to pass really small additions), or put them together into a single debugging support paper?

-- René Ferdinand Rivera Morell
-- Don't Assume Anything -- No Supone Nada
-- Robot Dreams - http://robot-dreams.net