[1/2] ld: Add --pdb option
Commit Message
This patch adds the --pdb option to ld when linking PE files, which
augments the existing CodeView record used for build IDs by adding a PDB
filename. If no filename is provided, this defaults to the image name
with the extension replaced by "pdb".
---
bfd/libpei.h | 6 +++--
bfd/peXXigen.c | 30 ++++++++++++++++-----
bfd/peicode.h | 2 +-
ld/emultempl/pe.em | 49 +++++++++++++++++++++++++++++++----
ld/emultempl/pep.em | 47 ++++++++++++++++++++++++++++++---
ld/testsuite/ld-pe/pdb.exp | 53 ++++++++++++++++++++++++++++++++++++++
ld/testsuite/ld-pe/pdb1.s | 5 ++++
7 files changed, 173 insertions(+), 19 deletions(-)
create mode 100644 ld/testsuite/ld-pe/pdb.exp
create mode 100644 ld/testsuite/ld-pe/pdb1.s
Comments
Hi Mark,
On Mon, 3 Oct 2022, Mark Harmstone wrote:
> This patch adds the --pdb option to ld when linking PE files, which
> augments the existing CodeView record used for build IDs by adding a PDB
> filename. If no filename is provided, this defaults to the image name
> with the extension replaced by "pdb".
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!
The testcase only seemed to exercise the form --pdb=<explicitname>, so I
thought I'd ask just for clarity: I guess the other forms of specifying
the option, e.g. "--pdb <explicitname>" also does the same - same thing
for setting the option with just one leading dash, "-pdb=<explicitname>"
(I guess it's a getopt feature that allows that as long as it isn't
ambiguous with single-letter options?), as well as the form "-pdb=" or
"--pdb=" for requesting it to set the default name?
// Martin
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.
> The testcase only seemed to exercise the form --pdb=<explicitname>, so I thought I'd ask just for clarity: I guess the other forms of specifying the option, e.g. "--pdb <explicitname>" also does the same - same thing for setting the option with just one leading dash, "-pdb=<explicitname>" (I guess it's a getopt feature that allows that as long as it isn't ambiguous with single-letter options?), as well as the form "-pdb=" or "--pdb=" for requesting it to set the default name?
Apparently provided that the option doesn't begin with an O, you can use either single dashes or double dashes. It looks like the equals sign is mandatory when providing optional parameters, otherwise it interprets the filename as another parameter. 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.
Mark
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.
// Martin
On Mon, Oct 03, 2022 at 02:43:12AM +0100, Mark Harmstone wrote:
> @@ -955,6 +960,13 @@ gld${EMULATION_NAME}_handle_option (int optc)
> if (strcmp (optarg, "none"))
> emit_build_id = xstrdup (optarg);
> break;
> + case OPTION_PDB:
> + if (emit_build_id == NULL)
> + emit_build_id = xstrdup (DEFAULT_BUILD_ID_STYLE);
> + pdb = 1;
> + if (optarg)
> + pdb_name = xstrdup (optarg);
> + break;
This will result in "--emit-build-id=none --pdb" enabling build-id
while "--pdb --emit-build-id=none" will disable build-id. Is that
what you want?
If the intent is to always force build-id on then you should do so in
after_parse instead. Otherwise both patches look OK to me.
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
On 7/10/22 13:16, Martin Storsjö wrote:
> 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
Hi Martin,
Fair enough - I'm not overly wedded to this, and will change it if, as you say, it'll cause issues elsewhere.
Mark
On Mon, 10 Oct 2022, Mark Harmstone wrote:
> On 7/10/22 13:16, Martin Storsjö wrote:
>> 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
> Hi Martin,
>
> Fair enough - I'm not overly wedded to this, and will change it if, as you
> say, it'll cause issues elsewhere.
Ok, great, thanks!
However this patchset also lost the ability to get an automatically chosen
output file name, which currently is used via the slightly awkward syntax
"--pdb=" without an empty parameter.
I see you refactored a bit of code in this revision of the patch, which
lost that ability. With the patch I'm attaching, applied on top of v1 of
your patch, I think it behaves as a reasonable compromise; getopt's
required_argument does allow the --pdb=<name> form too (which I think is
the one we still should recommend going forward), and passing "--pdb="
allows implying the automatic naming behaviour.
// Martin
Sorry Martin, I didn't quite get what you meant before. Yes, that's fine; I'll
resubmit with your changes.
Mark
On Mon, 10 Oct 2022, Mark Harmstone wrote:
> Sorry Martin, I didn't quite get what you meant before. Yes, that's
> fine; I'll resubmit with your changes.
Ok, great!
So after this, both lld and binutils would support these usage forms:
Preferred ones:
-Wl,--pdb=<filename>
-Wl,--pdb= (implying the PDB filename from the output name)
Also supported (and used across third party code) but less ideal:
-Wl,--pdb,<filename>
We could try to discourage (soft-deprecate?) the latter form and try to
change most accessible third party projects to use the former form which
is less ambiguous.
If we get most projects switched to using the preferred forms, and enough
time passes (say a year or two?) we could maybe consider to remove support
for the other form, leaving the door open for taking that into use as
"-Wl,--pdb" as a less awkward form for the implicit output name, at some
point in the future.
// Martin
@@ -388,9 +388,11 @@ void _bfd_XX_get_symbol_info (bfd *, asymbol *, symbol_info *);
bool _bfd_XXi_final_link_postscript (bfd *, struct coff_final_link_info *);
void _bfd_XXi_swap_debugdir_in (bfd *, void *, void *);
unsigned _bfd_XXi_swap_debugdir_out (bfd *, void *, void *);
-unsigned _bfd_XXi_write_codeview_record (bfd *, file_ptr, CODEVIEW_INFO *);
+unsigned _bfd_XXi_write_codeview_record
+ (bfd *, file_ptr, CODEVIEW_INFO *, const char *);
CODEVIEW_INFO *_bfd_XXi_slurp_codeview_record
- (bfd * abfd, file_ptr where, unsigned long length, CODEVIEW_INFO *cvinfo);
+ (bfd * abfd, file_ptr where, unsigned long length, CODEVIEW_INFO *cvinfo,
+ char **pdb);
/* The following are needed only for ONE of pe or pei, but don't
otherwise vary; peicode.h fixes up ifdefs but we provide the
@@ -1134,7 +1134,8 @@ _bfd_XXi_swap_debugdir_out (bfd * abfd, void * inp, void * extp)
}
CODEVIEW_INFO *
-_bfd_XXi_slurp_codeview_record (bfd * abfd, file_ptr where, unsigned long length, CODEVIEW_INFO *cvinfo)
+_bfd_XXi_slurp_codeview_record (bfd * abfd, file_ptr where, unsigned long length, CODEVIEW_INFO *cvinfo,
+ char **pdb)
{
char buffer[256+1];
bfd_size_type nread;
@@ -1174,6 +1175,9 @@ _bfd_XXi_slurp_codeview_record (bfd * abfd, file_ptr where, unsigned long length
cvinfo->SignatureLength = CV_INFO_SIGNATURE_LENGTH;
/* cvinfo->PdbFileName = cvinfo70->PdbFileName; */
+ if (pdb)
+ *pdb = xstrdup (cvinfo70->PdbFileName);
+
return cvinfo;
}
else if ((cvinfo->CVSignature == CVINFO_PDB20_CVSIGNATURE)
@@ -1185,6 +1189,9 @@ _bfd_XXi_slurp_codeview_record (bfd * abfd, file_ptr where, unsigned long length
cvinfo->SignatureLength = 4;
/* cvinfo->PdbFileName = cvinfo20->PdbFileName; */
+ if (pdb)
+ *pdb = xstrdup (cvinfo20->PdbFileName);
+
return cvinfo;
}
@@ -1192,9 +1199,11 @@ _bfd_XXi_slurp_codeview_record (bfd * abfd, file_ptr where, unsigned long length
}
unsigned int
-_bfd_XXi_write_codeview_record (bfd * abfd, file_ptr where, CODEVIEW_INFO *cvinfo)
+_bfd_XXi_write_codeview_record (bfd * abfd, file_ptr where, CODEVIEW_INFO *cvinfo,
+ const char *pdb)
{
- const bfd_size_type size = sizeof (CV_INFO_PDB70) + 1;
+ size_t pdb_len = pdb ? strlen (pdb) : 0;
+ const bfd_size_type size = sizeof (CV_INFO_PDB70) + pdb_len + 1;
bfd_size_type written;
CV_INFO_PDB70 *cvinfo70;
char * buffer;
@@ -1217,7 +1226,11 @@ _bfd_XXi_write_codeview_record (bfd * abfd, file_ptr where, CODEVIEW_INFO *cvinf
memcpy (&(cvinfo70->Signature[8]), &(cvinfo->Signature[8]), 8);
H_PUT_32 (abfd, cvinfo->Age, cvinfo70->Age);
- cvinfo70->PdbFileName[0] = '\0';
+
+ if (pdb == NULL)
+ cvinfo70->PdbFileName[0] = '\0';
+ else
+ memcpy (cvinfo70->PdbFileName, pdb, pdb_len + 1);
written = bfd_bwrite (buffer, size, abfd);
@@ -2615,22 +2628,25 @@ pe_print_debugdata (bfd * abfd, void * vfile)
We need to use a 32-bit aligned buffer
to safely read in a codeview record. */
char buffer[256 + 1] ATTRIBUTE_ALIGNED_ALIGNOF (CODEVIEW_INFO);
+ char *pdb;
CODEVIEW_INFO *cvinfo = (CODEVIEW_INFO *) buffer;
/* The debug entry doesn't have to have to be in a section,
in which case AddressOfRawData is 0, so always use PointerToRawData. */
if (!_bfd_XXi_slurp_codeview_record (abfd, (file_ptr) idd.PointerToRawData,
- idd.SizeOfData, cvinfo))
+ idd.SizeOfData, cvinfo, &pdb))
continue;
for (j = 0; j < cvinfo->SignatureLength; j++)
sprintf (&signature[j*2], "%02x", cvinfo->Signature[j] & 0xff);
/* xgettext:c-format */
- fprintf (file, _("(format %c%c%c%c signature %s age %ld)\n"),
+ fprintf (file, _("(format %c%c%c%c signature %s age %ld pdb %s)\n"),
buffer[0], buffer[1], buffer[2], buffer[3],
- signature, cvinfo->Age);
+ signature, cvinfo->Age, pdb[0] ? pdb : "(none)");
+
+ free (pdb);
}
}
@@ -1383,7 +1383,7 @@ pe_bfd_read_buildid (bfd *abfd)
*/
if (_bfd_XXi_slurp_codeview_record (abfd,
(file_ptr) idd.PointerToRawData,
- idd.SizeOfData, cvinfo))
+ idd.SizeOfData, cvinfo, NULL))
{
struct bfd_build_id* build_id = bfd_alloc (abfd,
sizeof (struct bfd_build_id) + cvinfo->SignatureLength);
@@ -146,6 +146,8 @@ static lang_assignment_statement_type *image_base_statement = 0;
static unsigned short pe_dll_characteristics = DEFAULT_DLL_CHARACTERISTICS;
static bool insert_timestamp = true;
static const char *emit_build_id;
+static int pdb;
+static char *pdb_name;
#ifdef DLL_SUPPORT
static int pe_enable_stdcall_fixup = -1; /* 0=disable 1=enable. */
@@ -284,7 +286,8 @@ fragment <<EOF
#define OPTION_INSERT_TIMESTAMP (OPTION_TERMINAL_SERVER_AWARE + 1)
#define OPTION_NO_INSERT_TIMESTAMP (OPTION_INSERT_TIMESTAMP + 1)
#define OPTION_BUILD_ID (OPTION_NO_INSERT_TIMESTAMP + 1)
-#define OPTION_ENABLE_RELOC_SECTION (OPTION_BUILD_ID + 1)
+#define OPTION_PDB (OPTION_BUILD_ID + 1)
+#define OPTION_ENABLE_RELOC_SECTION (OPTION_PDB + 1)
#define OPTION_DISABLE_RELOC_SECTION (OPTION_ENABLE_RELOC_SECTION + 1)
/* DLL Characteristics flags. */
#define OPTION_DISABLE_DYNAMIC_BASE (OPTION_DISABLE_RELOC_SECTION + 1)
@@ -383,6 +386,7 @@ gld${EMULATION_NAME}_add_options
{"tsaware", no_argument, NULL, OPTION_TERMINAL_SERVER_AWARE},
{"disable-tsaware", no_argument, NULL, OPTION_DISABLE_TERMINAL_SERVER_AWARE},
{"build-id", optional_argument, NULL, OPTION_BUILD_ID},
+ {"pdb", optional_argument, NULL, OPTION_PDB},
{"enable-reloc-section", no_argument, NULL, OPTION_ENABLE_RELOC_SECTION},
{"disable-reloc-section", no_argument, NULL, OPTION_DISABLE_RELOC_SECTION},
{NULL, no_argument, NULL, 0}
@@ -532,6 +536,7 @@ gld${EMULATION_NAME}_list_options (FILE *file)
fprintf (file, _(" --[disable-]wdmdriver Driver uses the WDM model\n"));
fprintf (file, _(" --[disable-]tsaware Image is Terminal Server aware\n"));
fprintf (file, _(" --build-id[=STYLE] Generate build ID\n"));
+ fprintf (file, _(" --pdb[=FILENAME] Generate PDB file\n"));
}
@@ -955,6 +960,13 @@ gld${EMULATION_NAME}_handle_option (int optc)
if (strcmp (optarg, "none"))
emit_build_id = xstrdup (optarg);
break;
+ case OPTION_PDB:
+ if (emit_build_id == NULL)
+ emit_build_id = xstrdup (DEFAULT_BUILD_ID_STYLE);
+ pdb = 1;
+ if (optarg)
+ pdb_name = xstrdup (optarg);
+ break;
}
/* Set DLLCharacteristics bits */
@@ -1256,6 +1268,7 @@ write_build_id (bfd *abfd)
bfd_size_type size;
bfd_size_type build_id_size;
unsigned char *build_id;
+ const char *pdb_base_name = NULL;
/* Find the section the .buildid output section has been merged info. */
for (asec = abfd->sections; asec != NULL; asec = asec->next)
@@ -1295,6 +1308,9 @@ write_build_id (bfd *abfd)
bfd_vma ib = pe_data (link_info.output_bfd)->pe_opthdr.ImageBase;
+ if (pdb_name)
+ pdb_base_name = lbasename (pdb_name);
+
/* Construct a debug directory entry which points to an immediately following CodeView record. */
struct internal_IMAGE_DEBUG_DIRECTORY idd;
idd.Characteristics = 0;
@@ -1302,7 +1318,7 @@ write_build_id (bfd *abfd)
idd.MajorVersion = 0;
idd.MinorVersion = 0;
idd.Type = PE_IMAGE_DEBUG_TYPE_CODEVIEW;
- idd.SizeOfData = sizeof (CV_INFO_PDB70) + 1;
+ idd.SizeOfData = sizeof (CV_INFO_PDB70) + (pdb_base_name ? strlen (pdb_base_name) : 0) + 1;
idd.AddressOfRawData = asec->vma - ib + link_order->offset
+ sizeof (struct external_IMAGE_DEBUG_DIRECTORY);
idd.PointerToRawData = asec->filepos + link_order->offset
@@ -1331,7 +1347,8 @@ write_build_id (bfd *abfd)
free (build_id);
/* Write the codeview record. */
- if (_bfd_XXi_write_codeview_record (abfd, idd.PointerToRawData, &cvinfo) == 0)
+ if (_bfd_XXi_write_codeview_record (abfd, idd.PointerToRawData, &cvinfo,
+ pdb_base_name) == 0)
return 0;
/* Record the location of the debug directory in the data directory. */
@@ -1368,11 +1385,14 @@ setup_build_id (bfd *ibfd)
/* Section is a fixed size:
One IMAGE_DEBUG_DIRECTORY entry, of type IMAGE_DEBUG_TYPE_CODEVIEW,
- pointing at a CV_INFO_PDB70 record containing the build-id, with a
- null byte for PdbFileName. */
+ pointing at a CV_INFO_PDB70 record containing the build-id, followed by
+ PdbFileName if relevant. */
s->size = sizeof (struct external_IMAGE_DEBUG_DIRECTORY)
+ sizeof (CV_INFO_PDB70) + 1;
+ if (pdb_name)
+ s->size += strlen (pdb_name);
+
return true;
}
@@ -1403,6 +1423,25 @@ gld${EMULATION_NAME}_after_open (void)
}
#endif
+ if (pdb && !pdb_name)
+ {
+ const char *base = lbasename (bfd_get_filename (link_info.output_bfd));
+ size_t len = strlen (base);
+ static const char suffix[] = ".pdb";
+
+ while (len > 0 && base[len] != '.')
+ {
+ len--;
+ }
+
+ if (len == 0)
+ len = strlen (base);
+
+ pdb_name = xmalloc (len + sizeof (suffix));
+ memcpy (pdb_name, base, len);
+ memcpy (pdb_name + len, suffix, sizeof (suffix));
+ }
+
if (emit_build_id != NULL)
{
bfd *abfd;
@@ -157,6 +157,8 @@ static lang_assignment_statement_type *image_base_statement = 0;
static unsigned short pe_dll_characteristics = DEFAULT_DLL_CHARACTERISTICS;
static bool insert_timestamp = true;
static const char *emit_build_id;
+static int pdb;
+static char *pdb_name;
#ifdef DLL_SUPPORT
static int pep_enable_stdcall_fixup = 1; /* 0=disable 1=enable (default). */
@@ -255,6 +257,7 @@ enum options
OPTION_NO_INSERT_TIMESTAMP,
OPTION_TERMINAL_SERVER_AWARE,
OPTION_BUILD_ID,
+ OPTION_PDB,
OPTION_ENABLE_RELOC_SECTION,
OPTION_DISABLE_RELOC_SECTION,
OPTION_DISABLE_HIGH_ENTROPY_VA,
@@ -343,6 +346,7 @@ gld${EMULATION_NAME}_add_options
{"insert-timestamp", no_argument, NULL, OPTION_INSERT_TIMESTAMP},
{"no-insert-timestamp", no_argument, NULL, OPTION_NO_INSERT_TIMESTAMP},
{"build-id", optional_argument, NULL, OPTION_BUILD_ID},
+ {"pdb", optional_argument, NULL, OPTION_PDB},
{"enable-reloc-section", no_argument, NULL, OPTION_ENABLE_RELOC_SECTION},
{"disable-reloc-section", no_argument, NULL, OPTION_DISABLE_RELOC_SECTION},
{"disable-high-entropy-va", no_argument, NULL, OPTION_DISABLE_HIGH_ENTROPY_VA},
@@ -490,6 +494,7 @@ gld${EMULATION_NAME}_list_options (FILE *file)
fprintf (file, _(" --[disable-]wdmdriver Driver uses the WDM model\n"));
fprintf (file, _(" --[disable-]tsaware Image is Terminal Server aware\n"));
fprintf (file, _(" --build-id[=STYLE] Generate build ID\n"));
+ fprintf (file, _(" --pdb[=FILENAME] Generate PDB file\n"));
#endif
}
@@ -898,6 +903,13 @@ gld${EMULATION_NAME}_handle_option (int optc)
if (strcmp (optarg, "none"))
emit_build_id = xstrdup (optarg);
break;
+ case OPTION_PDB:
+ if (emit_build_id == NULL)
+ emit_build_id = xstrdup (DEFAULT_BUILD_ID_STYLE);
+ pdb = 1;
+ if (optarg)
+ pdb_name = xstrdup (optarg);
+ break;
}
/* Set DLLCharacteristics bits */
@@ -1240,6 +1252,7 @@ write_build_id (bfd *abfd)
bfd_size_type size;
bfd_size_type build_id_size;
unsigned char *build_id;
+ const char *pdb_base_name = NULL;
/* Find the section the .buildid output section has been merged info. */
for (asec = abfd->sections; asec != NULL; asec = asec->next)
@@ -1279,6 +1292,9 @@ write_build_id (bfd *abfd)
bfd_vma ib = pe_data (link_info.output_bfd)->pe_opthdr.ImageBase;
+ if (pdb_name)
+ pdb_base_name = lbasename (pdb_name);
+
/* Construct a debug directory entry which points to an immediately following CodeView record. */
struct internal_IMAGE_DEBUG_DIRECTORY idd;
idd.Characteristics = 0;
@@ -1286,7 +1302,7 @@ write_build_id (bfd *abfd)
idd.MajorVersion = 0;
idd.MinorVersion = 0;
idd.Type = PE_IMAGE_DEBUG_TYPE_CODEVIEW;
- idd.SizeOfData = sizeof (CV_INFO_PDB70) + 1;
+ idd.SizeOfData = sizeof (CV_INFO_PDB70) + (pdb_base_name ? strlen (pdb_base_name) : 0) + 1;
idd.AddressOfRawData = asec->vma - ib + link_order->offset
+ sizeof (struct external_IMAGE_DEBUG_DIRECTORY);
idd.PointerToRawData = asec->filepos + link_order->offset
@@ -1315,7 +1331,8 @@ write_build_id (bfd *abfd)
free (build_id);
/* Write the codeview record. */
- if (_bfd_XXi_write_codeview_record (abfd, idd.PointerToRawData, &cvinfo) == 0)
+ if (_bfd_XXi_write_codeview_record (abfd, idd.PointerToRawData, &cvinfo,
+ pdb_base_name) == 0)
return 0;
/* Record the location of the debug directory in the data directory. */
@@ -1352,11 +1369,14 @@ setup_build_id (bfd *ibfd)
/* Section is a fixed size:
One IMAGE_DEBUG_DIRECTORY entry, of type IMAGE_DEBUG_TYPE_CODEVIEW,
- pointing at a CV_INFO_PDB70 record containing the build-id, with a
- null byte for PdbFileName. */
+ pointing at a CV_INFO_PDB70 record containing the build-id, followed by
+ PdbFileName if relevant. */
s->size = sizeof (struct external_IMAGE_DEBUG_DIRECTORY)
+ sizeof (CV_INFO_PDB70) + 1;
+ if (pdb_name)
+ s->size += strlen (pdb_name);
+
return true;
}
@@ -1388,6 +1408,25 @@ gld${EMULATION_NAME}_after_open (void)
}
#endif
+ if (pdb && !pdb_name)
+ {
+ const char *base = lbasename (bfd_get_filename (link_info.output_bfd));
+ size_t len = strlen (base);
+ static const char suffix[] = ".pdb";
+
+ while (len > 0 && base[len] != '.')
+ {
+ len--;
+ }
+
+ if (len == 0)
+ len = strlen (base);
+
+ pdb_name = xmalloc (len + sizeof (suffix));
+ memcpy (pdb_name, base, len);
+ memcpy (pdb_name + len, suffix, sizeof (suffix));
+ }
+
if (emit_build_id != NULL)
{
bfd *abfd;
new file mode 100644
@@ -0,0 +1,53 @@
+# Expect script for creating PDB files when linking.
+# Copyright (C) 2022 Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+if {![istarget i*86-*-mingw*]
+ && ![istarget x86_64-*-mingw*]} {
+ return
+}
+
+proc get_pdb_name { pe } {
+ global OBJDUMP
+
+ set exec_output [run_host_cmd "$OBJDUMP" "-p $pe"]
+
+ if ![regexp -line "^\\(format RSDS signature (\[0-9a-fA-F\]{32}) age 1 pdb (.*)\\)$" $exec_output full sig pdb] {
+ return ""
+ }
+
+ return $pdb
+}
+
+if ![ld_assemble $as $srcdir/$subdir/pdb1.s tmpdir/pdb1.o] {
+ unsupported "Build pdb1.o"
+ return
+}
+
+if ![ld_link $ld "tmpdir/pdb1.exe" "--pdb=tmpdir/pdb1.pdb tmpdir/pdb1.o"] {
+ fail "Could not create a PE image with a PDB file"
+ return
+}
+
+if ![string equal [get_pdb_name "tmpdir/pdb1.exe"] "pdb1.pdb"] {
+ fail "PDB filename not found in CodeView debug info"
+ return
+}
+
+pass "PDB filename present in CodeView debug info"
new file mode 100644
@@ -0,0 +1,5 @@
+.text
+
+.global foo
+foo:
+ .long 0x12345678