From: "Martin Storsjö" <martin@martin.st>
To: Mark Harmstone <mark@harmstone.com>
Cc: binutils@sourceware.org
Subject: Re: [PATCH 1/2] ld: Add --pdb option
Date: Fri, 7 Oct 2022 15:16:01 +0300 (EEST) [thread overview]
Message-ID: <2b3caba0-de53-6c2c-1038-5581deb2b8e@martin.st> (raw)
In-Reply-To: <7e5d88ff-9aa9-dbfd-aa80-1793c2c48fde@martin.st>
[-- Attachment #1: Type: text/plain, Size: 4163 bytes --]
On Mon, 3 Oct 2022, Martin Storsjö wrote:
> On Mon, 3 Oct 2022, Mark Harmstone wrote:
>
>> Hi Martin,
>>
>>> As I assume you're aware, lld's mingw port also supports PDB generation -
>>> and the description of this option also sounds like it's chosen to match
>>> lld's option for outputting PDB files - that's good!
>>
>> Yes, that's right. One notable difference is that the parameter here is
>> optional, unlike with lld, making it a lot easier to fit this into e.g.
>> CMake toolchain files or LDFLAGS.
>
> LLD also has got that behaviour, since
> https://github.com/llvm/llvm-project/commit/2c52ddf31f5421c5373923535b958b84c79772e3
> in 2019. That's in particular why I wanted to make sure that this case works
> the same in binutils too.
>
>> It looks like the equals sign is mandatory when providing optional
>> parameters, otherwise it interprets the filename as another parameter.
>
> Yep, that's the case in LLD too.
>
> Unfortunately I didn't think of this behaviour initially when I first added
> this option - otherwise we could have had e.g. --pdb as a boolean option to
> just output to the default name, and e.g. --output-pdb=<name> if you wanted
> to specify the name. But oh well, "-pdb=" works, and I guess it isn't the
> worst thing in the world.
>
>> But it does mean that the form "-pdb=out.pdb" will work on both ld and lld,
>> which I think is the most important thing.
>
> TBH, I consider the "-pdb=" case equally important too - that's what most
> people would use in the end.
FWIW, I'm actually a bit concerned about the interop between binutils and
lld here. I don't want interop between binutils and lld to work only for
some subset of the used parameter forms, I'd like it to work for all
commonly used forms.
First off, the (slightly awkward) syntax that lld uses for an optional
empty output name, "-pdb=" really should be handled by binutils too -
handling that doesn't conflict with anything else and should be simple to
support.
This is the format of the option that I've been recommending people to
use, and this has been in use in third party projects for years already -
e.g. this:
https://code.videolan.org/videolan/vlc/-/blob/master/configure.ac#L429
This should be trivial to support in your patch:
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index 11216830dd3..538fdf5054b 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -926,7 +926,7 @@ gld${EMULATION_NAME}_handle_option (int optc)
if (emit_build_id == NULL)
emit_build_id = xstrdup (DEFAULT_BUILD_ID_STYLE);
pdb = 1;
- if (optarg)
+ if (optarg && optarg[0])
pdb_name = xstrdup (optarg);
break;
}
(And the same for pe.em.)
Secondly, for explicitly naming an output file, I've documented to end
users that they can use either -Wl,-pdb=<filename> or -Wl,-pdb,<filename>
- see
https://github.com/mstorsjo/llvm-mingw/blob/master/README.md?plain=1#L175.
In the original implementation in the mingw frontend in lld in 2018, the
"-pdb <output>" format was the only format for the option:
https://github.com/llvm/llvm-project/commit/b7d50115ba4900da6db7afb6460ad42ff19ba6a2
Only one year later with the implicit output name, the "-pdb=<output>" and
"-pdb=" form was added:
https://github.com/llvm/llvm-project/commit/2c52ddf31f5421c5373923535b958b84c79772e3
In one of my test scripts, I use the initial form of the option,
-Wl,-pdb,<filename>:
https://github.com/mstorsjo/llvm-mingw/blob/master/run-tests.sh#L234
It seems like Wine has picked up on the -Wl,-pdb,<name> form:
https://gitlab.winehq.org/wine/wine/-/blob/wine-7.18/tools/winegcc/winegcc.c#L467
Also here are a couple of other cases I found that all seem to use that
form:
https://youtrack.jetbrains.com/issue/KT-47175/How-to-generate-kotlin-native-debug-info-filesPDB-on-windows-platform
https://git.kernel.dk/?p=fio.git;a=commitdiff;h=76bc30ca118fda404f19c17d97bafdba9779c4c2
So with all these users, I'd be kinda hesitant to change lld's
interpretation of this option form, and to have binutils ld in parallel
interpreting that form differently. What do you think?
// Martin
next prev parent reply other threads:[~2022-10-07 12:16 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-03 1:43 Mark Harmstone
2022-10-03 1:43 ` [PATCH 2/2] ld: Add minimal pdb generation Mark Harmstone
2022-10-03 5:12 ` [PATCH 1/2] ld: Add --pdb option Martin Storsjö
2022-10-03 16:57 ` Mark Harmstone
2022-10-03 18:58 ` Martin Storsjö
2022-10-07 12:16 ` Martin Storsjö [this message]
2022-10-09 23:46 ` Mark Harmstone
2022-10-10 10:27 ` Martin Storsjö
2022-10-10 16:55 ` Mark Harmstone
2022-10-10 20:58 ` Martin Storsjö
2022-10-05 4:20 ` Alan Modra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2b3caba0-de53-6c2c-1038-5581deb2b8e@martin.st \
--to=martin@martin.st \
--cc=binutils@sourceware.org \
--cc=mark@harmstone.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).