Page MenuHomePhabricator

[XCOFF] make .file directive have directory info
ClosedPublic

Authored by shchenz on Apr 1 2021, 8:53 PM.

Details

Summary

The .file directive is changed to only have basename in D36018 for ELF.

But on AIX, we require the .file directive to also contain the directory info. This aligns with other AIX compiler like XLC and is required by some AIX tool like DBX.

Diff Detail

Event Timeline

shchenz created this revision.Apr 1 2021, 8:53 PM
shchenz requested review of this revision.Apr 1 2021, 8:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2021, 8:53 PM
shchenz edited the summary of this revision. (Show Details)Apr 1 2021, 9:14 PM
shchenz updated this revision to Diff 334891.Apr 1 2021, 9:16 PM

Lint warning fix

From what I understand, this is meant to fix a situation where dbx is unhappy when the file is not simply compiled in the directory where it resides (using its basename). However, this unhappiness extends even to the compiled-using-basename case when the filename contains "special characters".

For example, a file with a tab character ends up with backslash escapes in the assembly (but the assembler on AIX doesn't interpret these):

$ clang -mllvm -dwarf-inlined-strings=Enable a\    q.c -S -o - | grep -F .file
        .file   "a\tq.c"

From what I understand, this is meant to fix a situation where dbx is unhappy when the file is not simply compiled in the directory where it resides (using its basename). However, this unhappiness extends even to the compiled-using-basename case when the filename contains "special characters".

For example, a file with a tab character ends up with backslash escapes in the assembly (but the assembler on AIX doesn't interpret these):

$ clang -mllvm -dwarf-inlined-strings=Enable a\    q.c -S -o - | grep -F .file
        .file   "a\tq.c"

@hubert.reinterpretcast thanks for providing the example. Yes, it is indeed an issue for the same name in symbol table and in dwinfo section.
As you can see in link https://www.ibm.com/docs/en/aix/7.2?topic=files-file-naming-conventions , special characters are not recommmended to be used in the file name, so can we just ignore the special characters and only handle the directory info in this patch? Thanks.

@hubert.reinterpretcast thanks for providing the example. Yes, it is indeed an issue for the same name in symbol table and in dwinfo section.
As you can see in link https://www.ibm.com/docs/en/aix/7.2?topic=files-file-naming-conventions , special characters are not recommmended to be used in the file name, so can we just ignore the special characters and only handle the directory info in this patch? Thanks.

The file name conventions on AIX does not (and probably cannot) recommend against the use of "national characters". The character escape mechanism being applied is incorrect. The string constants in AIX assembly are mostly-raw strings, with double quotation marks in the string content being immediately repeated to indicate that the end-of-string has not been reached.

llvm/test/CodeGen/PowerPC/aix-filename.ll
8 ↗(On Diff #334891)

There should be an additional test to check against the desired behaviour observed with relative paths (e.g., ../relative/path/to/file).

shchenz updated this revision to Diff 340457.Mon, Apr 26, 2:15 AM

1: add a relative path case

@hubert.reinterpretcast thanks for providing the example. Yes, it is indeed an issue for the same name in symbol table and in dwinfo section.
As you can see in link https://www.ibm.com/docs/en/aix/7.2?topic=files-file-naming-conventions , special characters are not recommmended to be used in the file name, so can we just ignore the special characters and only handle the directory info in this patch? Thanks.

The file name conventions on AIX does not (and probably cannot) recommend against the use of "national characters". The character escape mechanism being applied is incorrect. The string constants in AIX assembly are mostly-raw strings, with double quotation marks in the string content being immediately repeated to indicate that the end-of-string has not been reached.

Thanks, the special characters will be handled in patch D101280

shchenz marked an inline comment as done.Mon, Apr 26, 2:23 AM
This revision is now accepted and ready to land.Mon, Apr 26, 1:06 PM
This revision was landed with ongoing or failed builds.Mon, Apr 26, 9:15 PM
This revision was automatically updated to reflect the committed changes.