* [PATCH] gold: fix crash for empty file
@ 2021-11-19 14:01 Martin Liška
2021-11-29 9:00 ` Martin Liška
0 siblings, 1 reply; 5+ messages in thread
From: Martin Liška @ 2021-11-19 14:01 UTC (permalink / raw)
To: binutils; +Cc: ccoutant, Ian Lance Taylor
Hi.
The patch fixes a crash I noticed and explained in the linked PR.
Patch survives regression tests.
Ready to be installed?
Thanks,
Martin
gold/ChangeLog:
PR 28585
* symtab.cc (Symbol_table::lookup): Return NULL for NULL
argument.
---
gold/symtab.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 5a21ddc8cc2..ed6b5434592 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -701,6 +701,8 @@ Symbol_table::resolve_forwards(const Symbol* from) const
Symbol*
Symbol_table::lookup(const char* name, const char* version) const
{
+ if (name == NULL)
+ return NULL;
Stringpool::Key name_key;
name = this->namepool_.find(name, &name_key);
if (name == NULL)
--
2.33.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gold: fix crash for empty file
2021-11-19 14:01 [PATCH] gold: fix crash for empty file Martin Liška
@ 2021-11-29 9:00 ` Martin Liška
2022-01-05 10:15 ` Martin Liška
0 siblings, 1 reply; 5+ messages in thread
From: Martin Liška @ 2021-11-29 9:00 UTC (permalink / raw)
To: binutils; +Cc: Ian Lance Taylor
PING^1
On 11/19/21 15:01, Martin Liška wrote:
> Hi.
>
> The patch fixes a crash I noticed and explained in the linked PR.
>
> Patch survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> gold/ChangeLog:
>
> PR 28585
> * symtab.cc (Symbol_table::lookup): Return NULL for NULL
> argument.
> ---
> gold/symtab.cc | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/gold/symtab.cc b/gold/symtab.cc
> index 5a21ddc8cc2..ed6b5434592 100644
> --- a/gold/symtab.cc
> +++ b/gold/symtab.cc
> @@ -701,6 +701,8 @@ Symbol_table::resolve_forwards(const Symbol* from) const
> Symbol*
> Symbol_table::lookup(const char* name, const char* version) const
> {
> + if (name == NULL)
> + return NULL;
> Stringpool::Key name_key;
> name = this->namepool_.find(name, &name_key);
> if (name == NULL)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gold: fix crash for empty file
2021-11-29 9:00 ` Martin Liška
@ 2022-01-05 10:15 ` Martin Liška
2022-01-05 17:35 ` Cary Coutant
0 siblings, 1 reply; 5+ messages in thread
From: Martin Liška @ 2022-01-05 10:15 UTC (permalink / raw)
To: binutils; +Cc: Ian Lance Taylor
PING^2
On 11/29/21 10:00, Martin Liška wrote:
> PING^1
>
> On 11/19/21 15:01, Martin Liška wrote:
>> Hi.
>>
>> The patch fixes a crash I noticed and explained in the linked PR.
>>
>> Patch survives regression tests.
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> gold/ChangeLog:
>>
>> PR 28585
>> * symtab.cc (Symbol_table::lookup): Return NULL for NULL
>> argument.
>> ---
>> gold/symtab.cc | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/gold/symtab.cc b/gold/symtab.cc
>> index 5a21ddc8cc2..ed6b5434592 100644
>> --- a/gold/symtab.cc
>> +++ b/gold/symtab.cc
>> @@ -701,6 +701,8 @@ Symbol_table::resolve_forwards(const Symbol* from) const
>> Symbol*
>> Symbol_table::lookup(const char* name, const char* version) const
>> {
>> + if (name == NULL)
>> + return NULL;
>> Stringpool::Key name_key;
>> name = this->namepool_.find(name, &name_key);
>> if (name == NULL)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gold: fix crash for empty file
2022-01-05 10:15 ` Martin Liška
@ 2022-01-05 17:35 ` Cary Coutant
2022-01-06 8:35 ` Martin Liška
0 siblings, 1 reply; 5+ messages in thread
From: Cary Coutant @ 2022-01-05 17:35 UTC (permalink / raw)
To: Martin Liška; +Cc: Binutils, Ian Lance Taylor
> >> @@ -701,6 +701,8 @@ Symbol_table::resolve_forwards(const Symbol* from) const
> >> Symbol*
> >> Symbol_table::lookup(const char* name, const char* version) const
> >> {
> >> + if (name == NULL)
> >> + return NULL;
I'm more concerned with why a NULL name got passed to lookup in the
first place. In this test case, it's because parameters->entry() is
returning NULL, which in turn is because no (valid) object files have
been named, so a target hasn't been determined. I'd be more
comfortable with maintaining the requirement of passing a non-null
name to Symbol_table::lookup() (perhaps even adding an assert to that
effect), and then fixing Plugin_hook::run() to check for NULL before
calling lookup.
-cary
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gold: fix crash for empty file
2022-01-05 17:35 ` Cary Coutant
@ 2022-01-06 8:35 ` Martin Liška
0 siblings, 0 replies; 5+ messages in thread
From: Martin Liška @ 2022-01-06 8:35 UTC (permalink / raw)
To: Cary Coutant; +Cc: Binutils, Ian Lance Taylor
On 1/5/22 18:35, Cary Coutant wrote:
>>>> @@ -701,6 +701,8 @@ Symbol_table::resolve_forwards(const Symbol* from) const
>>>> Symbol*
>>>> Symbol_table::lookup(const char* name, const char* version) const
>>>> {
>>>> + if (name == NULL)
>>>> + return NULL;
>
> I'm more concerned with why a NULL name got passed to lookup in the
> first place. In this test case, it's because parameters->entry() is
> returning NULL, which in turn is because no (valid) object files have
> been named, so a target hasn't been determined. I'd be more
> comfortable with maintaining the requirement of passing a non-null
> name to Symbol_table::lookup() (perhaps even adding an assert to that
> effect), and then fixing Plugin_hook::run() to check for NULL before
> calling lookup.
Sure, I must confess I don't know gold enough. Thus, leaving that to you.
Cheers,
Martin
>
> -cary
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-06 8:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19 14:01 [PATCH] gold: fix crash for empty file Martin Liška
2021-11-29 9:00 ` Martin Liška
2022-01-05 10:15 ` Martin Liška
2022-01-05 17:35 ` Cary Coutant
2022-01-06 8:35 ` Martin Liška
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).