Re: [Tinycc-devel] [patch] adding path resolution to #line directives

[Michael Matz]

Hey,

On Fri, 6 May 2022, Raul Hernandez wrote:

> > It would seem better to canonicalize during generating this, because 
> > the above and this don't look equivalent anyway (they are equivalent 
> > only when the above relative path is less that seven levels deep from 
> > /).
>
> In our case this isn’t an issue, since the .c file is always compiled 
> under /tmp/v_*/, so two levels would actually be enough here.

Sure, but TCC also has to work on input not coming from V.

> The problem is that we cannot use an absolute path either (/home/…), 
> because tcc will always prepend the file’s directory, even if the #line 
> directive contains an absolute path. I guess a different fix could be to 
> check for absolute paths, and prevent tcc from prepending the file 
> surname if that’s the case,

That seems like a fix for what clearly is a bug in TCC, yes.  TCC 
shouldn't prepend something that is already an absolute path.

> but I still think resolving relative paths 
> is useful (one could generate #line directives like 
> /my/lang/stdlib/../thirdparty/foo.c), and tcc would resolve them down to 
> /my/lang/thirdparty/foo.c for cleaner backtraces, for example.

Well, IMHO TCC should do as little processing on what the producer put 
into the '#line' directives, simply because one has to assume that the 
producer did whatever it did for good reasons.  Prepending the compile 
directory to relative paths is borderline acceptably IMHO, but more than 
that: I don't think so.

> > So you simply remove '../' components without a matching directory 
> > part. That isn't correct in all circumstances.
>
> This is only ever hit when enough ../’s have been parsed so that the 
> beginning of the path (inserted by tcc or otherwise) has been completely 
> removed, and we’re left at the root of the filesystem. In that case, 
> /../ can be collapsed down to / . Could you give me an example of where 
> this would be incorrect?

Hmm, you're right, it should be okay.  (It still feels strange, as it 
relies on the fact that '/..' is equal to '/').  But that still leaves the 
issue of symlinked directories, removing "dir/../" components from paths 
simply isn't preserving the path when 'dir' is a symlink to another 
directory:

% mkdir -p sub/sub2; touch sub/sub2/foo
% ln -sf sub/sub2 symlink
% ls symlink/../sub2/foo
symlink/../sub2/foo
% ls sub2/foo
ls: cannot access 'sub2/foo': No such file or directory

So, no, I don't think the patch is okay, it can transform correct input 
into incorrect output, even if it sometimes also transforms into nicer 
output.

> > You might consider using realpath(3) (which is in POSIX) to do all of 
> > this for you.
>
> I tried that, but realpath() checks if the path actually exists on the 
> filesystem, and returns an error if it does not, unfortunately.

Well, yeah, it has to, due to the above symlink problem.

> > This might be generally usefull to libtcc users, and the variant 
> > printing it out can be written in terms of the char** variant; so that 
> > seems sensible to have.
>
> I can include that function then; the main__foo -> main.foo change is 
> indeed very V-specific.
>
> Related: libbacktrace has an API where it accepts a callback, which is 
> then called with a backtrace frame's information 
> (https://github.com/ianlancetaylor/libbacktrace/blob/master/backtrace.h#L91-L102 
> <https://github.com/ianlancetaylor/libbacktrace/blob/master/backtrace.h#L91-L102>), 
> so that might be a useful alternative too.

Maybe, yeah.  I don't have a strong opinion there.


Ciao,
Michael.