Why is Throwable.TraceInfo.toString not @safe?

[Johannes Loher]

Am 22.07.19 um 20:38 schrieb Jonathan M Davis:
> On Monday, July 22, 2019 1:29:21 AM MDT Johannes Loher via Digitalmars-d-
> learn wrote:
>> Am 22.07.19 um 05:16 schrieb Paul Backus:
>>> On Sunday, 21 July 2019 at 18:03:33 UTC, Johannes Loher wrote:
>>>> I'd like to log stacktraces of caught exceptions in an @safe manner.
>>>> However, Throwable.TraceInfo.toString is not @safe (or @trusted), so
>>>> this is not possible. Why is it not @safe? Can it be @trusted?
>>>>
>>>> Thanks for your help!
>>>
>>> Seems like it's because it uses the form of toString that accepts a
>>> delegate [1], and that delegate parameter is not marked as @safe.
>>>
>>> [1] https://dlang.org/phobos/object.html#.Throwable.toString.2
>>
>> I'm not talking about Throwable's toString method, but about
>> Throwable.TraceInfo's. Throwable.TraceInfo is an Interface inside
>> Throwable:
>>
>> interface TraceInfo
>> {
>>     int opApply(scope int delegate(ref const(char[]))) const;
>>     int opApply(scope int delegate(ref size_t, ref const(char[]))) const;
>>     string toString() const;
>> }
>>
>> Throwable has a member info of type TraceInfo. It is never explicitly
>> set, so I assume it is automatically set by runtime magic. This is the
>> constructor of Throwable:
>>
>> @nogc @safe pure nothrow this(string msg, Throwable nextInChain = null)
>> {
>>     this.msg = msg;
>>     this.nextInChain = nextInChain;
>>     //this.info = _d_traceContext();
>> }
>>
>> In theory people could define their own exception classes which provide
>> their own implementation of TraceInfo, but I never heard of anybody
>> doing this. So the real question is if the toString method of of the
>> DRuntime implementations (I assume there might be different
>> implementations for different platforms) are actually @safe (or
>> @trusted) and why we do not mark the interface to be @safe then.
> 
> All of that stuff predates @safe and most if not all attributes. @safe and
> the like have been added to some of that stuff, but in some cases, doing so
> would break code. I'd have to look at TraceInfo in more detail to know
> whether it can reasonably be marked @safe, but if it might end up calling
> anything that isn't guaranteed to be @safe, then it probably can't be (e.g.
> if it calls Object's toString). Also, TraceInfo is really more for
> druntime's use than for your typical programmer to use it directly in their
> program. So, I wouldn't be surprised in the least if not much effort was
> ever put in to making it @safe-friendly even if it could be. In general,
> stuff like @safe has tended to be applied to stuff in druntime and Phobos an
> a case-by-case basis, so it's really not surprising when an attribute is
> missing when it arguably should be there (though it frequently can't be
> there due to stuff like template arguments).
> 
> You _do_ need to be very careful with attributes and interfaces/classes
> though, because once an attribute is or isn't there, that can lock every
> derived class into a particular set of attributes. So, it's not always
> straightforward whether an attribute should be present or not. There are
> plenty of cases where ideally it would be present, but it can't be (e.g. a
> number of functions on TimeZone aren't pure even though they could be for
> _most_ derived classes, because they can't be pure for LocalTime), and there
> are cases where an attribute should be present but was simply never added. I
> don't know where TraceInfo sits, since I haven't dug into it.
> 
> - Jonathan M Davis
> 
> 
> 

Thanks for your insights, I already guessed that it is simply too old
for @safe and friends...

I understand that we have to be careful with adding attributes to
interfaces / classes, but in this particular case, I really cannot
imagine a usecase where we would not want it to be @safe (and the
situation here is better than with your example of TimeZone because we
have the @trusted escape hatch). Also I cannot imagine anybody
implementing their own version of TraceInfo.

The thing is that it can actually be quite useful to access TraceInfo in
user code, exactly for the example I gave in the beginning: Logging the
stacktrace. For long running programs, it is common practice in the
industry to log the stacktraces of caught exceptions in error cases
which do not mandate a program crash, simply to make a postmortem
analysis even possible. And in my opinion, user code should be @safe as
much as possible. If we cannot mark this @trusted, it means that a whole
lot of user code cannot be @safe, simply because of logging which would
be really weird...

I had a quick look at the actual implementations of TraceInfo in
DRuntime. There are 2 very simple implementations: SuppressTraceInfo (in
core/exception.d) and StackTrace (in core/sys/windows/stacktrace.d). In
both cases, toString can be @trusted. The first is simply a dummy
implementation (just returns null) and the second one is actuall marked
@trusted (and also has quite a simple implementation).

What gives me a headache is the 3rd implementation, DefaultTraceInfo (in
core/runtime.d). Its toString method is actually simple, but it only
calls its opApply implementation which is quite elaborate and definitely
not @safe (e.g. it does a lot of pointer arithmetic). But it is also
basically impossible for me to verify to be @trusted: It is simply too
long and calls too many low level functions :( However, I'd really
expect it to be @trusted. Everything else would be horrifying,
considering that the stacktrace gets printed whenever an exception is
not caught by the program. If this allowed arbitrary memory corruption
to happen, that would be really bad because it would mean that by
throwing an uncaught exception, anything could happen.