* Re: [PATCH 1/2] libdw: ensure read_encoded_value's value is set
@ 2015-02-11 23:22 Petr Machata
0 siblings, 0 replies; 4+ messages in thread
From: Petr Machata @ 2015-02-11 23:22 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 420 bytes --]
Josh Stone <jistone@redhat.com> writes:
> With CFLAGS='-Og -g', F21 gcc hits -Werror=maybe-uninitialized in
> read_encoded_value at "*result += value". It's fine with -O2/-O0.
>
> By my inspection, the only way those don't set value are for error
> cases, which will then return immediately.
Looks good to me. I don't see how we could end up using value
uninitialized either, but oh well.
Thanks,
Petr
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] libdw: ensure read_encoded_value's value is set
@ 2015-02-13 2:16 Josh Stone
0 siblings, 0 replies; 4+ messages in thread
From: Josh Stone @ 2015-02-13 2:16 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 942 bytes --]
On 02/12/2015 03:32 AM, Mark Wielaard wrote:
> On Thu, 2015-02-12 at 00:22 +0100, Petr Machata wrote:
>> Josh Stone <jistone@redhat.com> writes:
>>
>>> With CFLAGS='-Og -g', F21 gcc hits -Werror=maybe-uninitialized in
>>> read_encoded_value at "*result += value". It's fine with -O2/-O0.
>>>
>>> By my inspection, the only way those don't set value are for error
>>> cases, which will then return immediately.
>>
>> Looks good to me. I don't see how we could end up using value
>> uninitialized either, but oh well.
>
> Agreed. In general I don't like working around limitations in the
> compiler not correctly deducing things like this. Especially since it
> does seem to figure things out fine with -O0/-O2. But in both this case
> and the test case the explicit initialization seems harmless/not on a
> critical path. And it is nice to have -Og work for those that want to
> use it.
Thanks, both are pushed now.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] libdw: ensure read_encoded_value's value is set
@ 2015-02-12 11:32 Mark Wielaard
0 siblings, 0 replies; 4+ messages in thread
From: Mark Wielaard @ 2015-02-12 11:32 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 862 bytes --]
On Thu, 2015-02-12 at 00:22 +0100, Petr Machata wrote:
> Josh Stone <jistone@redhat.com> writes:
>
> > With CFLAGS='-Og -g', F21 gcc hits -Werror=maybe-uninitialized in
> > read_encoded_value at "*result += value". It's fine with -O2/-O0.
> >
> > By my inspection, the only way those don't set value are for error
> > cases, which will then return immediately.
>
> Looks good to me. I don't see how we could end up using value
> uninitialized either, but oh well.
Agreed. In general I don't like working around limitations in the
compiler not correctly deducing things like this. Especially since it
does seem to figure things out fine with -O0/-O2. But in both this case
and the test case the explicit initialization seems harmless/not on a
critical path. And it is nice to have -Og work for those that want to
use it.
Thanks,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] libdw: ensure read_encoded_value's value is set
@ 2015-02-11 22:27 Josh Stone
0 siblings, 0 replies; 4+ messages in thread
From: Josh Stone @ 2015-02-11 22:27 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]
With CFLAGS='-Og -g', F21 gcc hits -Werror=maybe-uninitialized in
read_encoded_value at "*result += value". It's fine with -O2/-O0.
In particular it seems to care about the __libdw_cfi_read_address_inc
calls. By my inspection, the only way those don't set value are for
error cases, which will then return immediately. This patch just sets
value = 0 to begin with, so gcc is always convinced it's fine.
Signed-off-by: Josh Stone <jistone@redhat.com>
---
libdw/ChangeLog | 4 ++++
libdw/encoded-value.h | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index d377937ff57c..b6ad425d8640 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,7 @@
+2015-02-11 Josh Stone <jistone@redhat.com>
+
+ * encoded-value.h (read_encoded_value): Initialize value.
+
2014-12-24 Mark Wielaard <mjw@redhat.com>
* dwarf_getsrc_die.c (dwarf_getsrc_die): Return the last line record
diff --git a/libdw/encoded-value.h b/libdw/encoded-value.h
index f953f5ef2a5f..0fa201835c5e 100644
--- a/libdw/encoded-value.h
+++ b/libdw/encoded-value.h
@@ -152,7 +152,7 @@ read_encoded_value (const Dwarf_CFI *cache, uint8_t encoding,
return true;
}
- Dwarf_Addr value;
+ Dwarf_Addr value = 0;
const unsigned char *endp = cache->data->d.d_buf + cache->data->d.d_size;
switch (encoding & 0x0f)
{
--
2.1.0
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-02-13 2:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-11 23:22 [PATCH 1/2] libdw: ensure read_encoded_value's value is set Petr Machata
-- strict thread matches above, loose matches on Subject: below --
2015-02-13 2:16 Josh Stone
2015-02-12 11:32 Mark Wielaard
2015-02-11 22:27 Josh Stone
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).