tinycc-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Michael Matz
Subject: Re: [Tinycc-devel] [patch] adding path resolution to #line directives
Date: Fri, 6 May 2022 15:46:26 +0200 (CEST)
User-agent: Alpine 2.21 (LSU 202 2017-01-01)

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]