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_at_[hidden]> wrote:

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