* Re: [PATCH] No nested functions in dwarf_get{srclines,scopevar}
@ 2015-10-14 19:44 Mark Wielaard
0 siblings, 0 replies; 9+ messages in thread
From: Mark Wielaard @ 2015-10-14 19:44 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 549 bytes --]
On Tue, 2015-10-13 at 15:39 -0700, Chih-Hung Hsieh wrote:
> Move nested functions in libdw/dwarf_get{scopevar,srclines}.c
> to file scope to compile with clang/llvm.
Since I would like to get a new release out on Friday and have a little
time to test things I am not immediately pushing these patches right
now. They are probably fine, but I like to look over them first. And
since they are somewhat large I am afraid we might introduce some subtle
bug right before the release. I'll get to them as soon as the release is
done on Friday.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] No nested functions in dwarf_get{srclines,scopevar}
@ 2015-11-13 21:21 Mark Wielaard
0 siblings, 0 replies; 9+ messages in thread
From: Mark Wielaard @ 2015-11-13 21:21 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 600 bytes --]
On Fri, 2015-11-13 at 18:10 +0100, Mark Wielaard wrote:
> On Thu, 2015-10-15 at 16:40 -0700, Chih-hung Hsieh wrote:
> > Please review the new attached 0002*patch file again.
>
> I pushed the dwarf_getscopevar part as attached. That one is small and
> seems correct. Still looking at the large changes in dwarf_getsrclines.
Also pushed the dwarf_getsrclines changes, with 3 small indentation
tweaks, to master as attached now. Sorry for splitting this into two.
But it made review easier and if we did introduce a bug by accident it
will be easier to track down later.
Thanks,
Mark
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-No-nested-functions-in-dwarf_getsrclines.patch --]
[-- Type: text/x-patch, Size: 14984 bytes --]
From f8443bd09f8a8d3d84a63e5ce206a218e57dff7a Mon Sep 17 00:00:00 2001
From: Chih-Hung Hsieh <chh@google.com>
Date: Tue, 13 Oct 2015 15:26:14 -0700
Subject: [PATCH] No nested functions in dwarf_getsrclines.
Move nested functions in libdw/dwarf_getsrclines.c to file scope.
Signed-off-by: Chih-Hung Hsieh <chh@google.com>
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
libdw/ChangeLog | 6 ++
libdw/dwarf_getsrclines.c | 243 ++++++++++++++++++++++++++--------------------
2 files changed, 144 insertions(+), 105 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index fc044a2..b344d92 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,5 +1,11 @@
2015-10-13 Chih-Hung Hsieh <chh@google.com>
+ * dwarf_getsrclines.c (read_srclines): Move nested functions
+ 'advance_pc' and 'add_new_line' to file scope and keep many
+ local state variables within one structure.
+
+2015-10-13 Chih-Hung Hsieh <chh@google.com>
+
* dwarf_getscopevar.c (dwarf_getscopevar): Move nested
function 'file_matches' to file scope.
diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c
index 389c824..03bdc8f 100644
--- a/libdw/dwarf_getsrclines.c
+++ b/libdw/dwarf_getsrclines.c
@@ -78,6 +78,74 @@ compare_lines (const void *a, const void *b)
: 0;
}
+struct line_state
+{
+ Dwarf_Word addr;
+ unsigned int op_index;
+ unsigned int file;
+ int64_t line;
+ unsigned int column;
+ uint_fast8_t is_stmt;
+ bool basic_block;
+ bool prologue_end;
+ bool epilogue_begin;
+ unsigned int isa;
+ unsigned int discriminator;
+ struct linelist *linelist;
+ size_t nlinelist;
+ unsigned int end_sequence;
+};
+
+static inline void
+run_advance_pc (struct line_state *state, unsigned int op_advance,
+ uint_fast8_t minimum_instr_len, uint_fast8_t max_ops_per_instr)
+{
+ state->addr += minimum_instr_len * ((state->op_index + op_advance)
+ / max_ops_per_instr);
+ state->op_index = (state->op_index + op_advance) % max_ops_per_instr;
+}
+
+static inline bool
+add_new_line (struct line_state *state, struct linelist *new_line)
+{
+ /* Set the line information. For some fields we use bitfields,
+ so we would lose information if the encoded values are too large.
+ Check just for paranoia, and call the data "invalid" if it
+ violates our assumptions on reasonable limits for the values. */
+ new_line->next = state->linelist;
+ new_line->sequence = state->nlinelist;
+ state->linelist = new_line;
+ ++(state->nlinelist);
+
+ /* Set the line information. For some fields we use bitfields,
+ so we would lose information if the encoded values are too large.
+ Check just for paranoia, and call the data "invalid" if it
+ violates our assumptions on reasonable limits for the values. */
+#define SET(field) \
+ do { \
+ new_line->line.field = state->field; \
+ if (unlikely (new_line->line.field != state->field)) \
+ return true; \
+ } while (0)
+
+ SET (addr);
+ SET (op_index);
+ SET (file);
+ SET (line);
+ SET (column);
+ SET (is_stmt);
+ SET (basic_block);
+ SET (end_sequence);
+ SET (prologue_end);
+ SET (epilogue_begin);
+ SET (isa);
+ SET (discriminator);
+
+#undef SET
+
+ return false;
+}
+
static int
read_srclines (Dwarf *dbg,
const unsigned char *linep, const unsigned char *lineendp,
@@ -86,8 +154,6 @@ read_srclines (Dwarf *dbg,
{
int res = -1;
- struct linelist *linelist = NULL;
- size_t nlinelist = 0;
size_t nfilelist = 0;
unsigned int ndirlist = 0;
@@ -323,27 +389,28 @@ read_srclines (Dwarf *dbg,
/* We are about to process the statement program. Initialize the
state machine registers (see 6.2.2 in the v2.1 specification). */
- Dwarf_Word addr = 0;
- unsigned int op_index = 0;
- unsigned int file = 1;
- /* We only store an int, but want to check for overflow (see SET below). */
- int64_t line = 1;
- unsigned int column = 0;
- uint_fast8_t is_stmt = default_is_stmt;
- bool basic_block = false;
- bool prologue_end = false;
- bool epilogue_begin = false;
- unsigned int isa = 0;
- unsigned int discriminator = 0;
+ struct line_state state =
+ {
+ .linelist = NULL,
+ .nlinelist = 0,
+ .addr = 0,
+ .op_index = 0,
+ .file = 1,
+ /* We only store int but want to check for overflow (see SET above). */
+ .line = 1,
+ .column = 0,
+ .is_stmt = default_is_stmt,
+ .basic_block = false,
+ .prologue_end = false,
+ .epilogue_begin = false,
+ .isa = 0,
+ .discriminator = 0
+ };
/* Apply the "operation advance" from a special opcode or
DW_LNS_advance_pc (as per DWARF4 6.2.5.1). */
- inline void advance_pc (unsigned int op_advance)
- {
- addr += minimum_instr_len * ((op_index + op_advance)
- / max_ops_per_instr);
- op_index = (op_index + op_advance) % max_ops_per_instr;
- }
+#define advance_pc(op_advance) \
+ run_advance_pc (&state, op_advance, minimum_instr_len, max_ops_per_instr)
/* Process the instructions. */
@@ -352,51 +419,16 @@ read_srclines (Dwarf *dbg,
struct linelist llstack[MAX_STACK_LINES];
#define NEW_LINE(end_seq) \
do { \
- struct linelist *ll = (nlinelist < MAX_STACK_LINES \
- ? &llstack[nlinelist] \
+ struct linelist *ll = (state.nlinelist < MAX_STACK_LINES \
+ ? &llstack[state.nlinelist] \
: malloc (sizeof (struct linelist))); \
if (unlikely (ll == NULL)) \
goto no_mem; \
- if (unlikely (add_new_line (ll, end_seq))) \
+ state.end_sequence = end_seq; \
+ if (unlikely (add_new_line (&state, ll))) \
goto invalid_data; \
} while (0)
- inline bool add_new_line (struct linelist *new_line, bool end_sequence)
- {
- new_line->next = linelist;
- new_line->sequence = nlinelist;
- linelist = new_line;
- ++nlinelist;
-
- /* Set the line information. For some fields we use bitfields,
- so we would lose information if the encoded values are too large.
- Check just for paranoia, and call the data "invalid" if it
- violates our assumptions on reasonable limits for the values. */
-#define SET(field) \
- do { \
- new_line->line.field = field; \
- if (unlikely (new_line->line.field != field)) \
- return true; \
- } while (0)
-
- SET (addr);
- SET (op_index);
- SET (file);
- SET (line);
- SET (column);
- SET (is_stmt);
- SET (basic_block);
- SET (end_sequence);
- SET (prologue_end);
- SET (epilogue_begin);
- SET (isa);
- SET (discriminator);
-
-#undef SET
-
- return false;
- }
-
while (linep < lineendp)
{
unsigned int opcode;
@@ -422,17 +454,17 @@ read_srclines (Dwarf *dbg,
+ (opcode - opcode_base) % line_range);
/* Perform the increments. */
- line += line_increment;
+ state.line += line_increment;
advance_pc ((opcode - opcode_base) / line_range);
/* Add a new line with the current state machine values. */
NEW_LINE (0);
/* Reset the flags. */
- basic_block = false;
- prologue_end = false;
- epilogue_begin = false;
- discriminator = 0;
+ state.basic_block = false;
+ state.prologue_end = false;
+ state.epilogue_begin = false;
+ state.discriminator = 0;
}
else if (opcode == 0)
{
@@ -457,28 +489,28 @@ read_srclines (Dwarf *dbg,
NEW_LINE (1);
/* Reset the registers. */
- addr = 0;
- op_index = 0;
- file = 1;
- line = 1;
- column = 0;
- is_stmt = default_is_stmt;
- basic_block = false;
- prologue_end = false;
- epilogue_begin = false;
- isa = 0;
- discriminator = 0;
+ state.addr = 0;
+ state.op_index = 0;
+ state.file = 1;
+ state.line = 1;
+ state.column = 0;
+ state.is_stmt = default_is_stmt;
+ state.basic_block = false;
+ state.prologue_end = false;
+ state.epilogue_begin = false;
+ state.isa = 0;
+ state.discriminator = 0;
break;
case DW_LNE_set_address:
/* The value is an address. The size is defined as
apporiate for the target machine. We use the
address size field from the CU header. */
- op_index = 0;
+ state.op_index = 0;
if (unlikely (lineendp - linep < (uint8_t) address_size))
goto invalid_data;
if (__libdw_read_address_inc (dbg, IDX_debug_line, &linep,
- address_size, &addr))
+ address_size, &state.addr))
goto out;
break;
@@ -542,7 +574,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
- get_uleb128 (discriminator, linep, lineendp);
+ get_uleb128 (state.discriminator, linep, lineendp);
break;
default:
@@ -567,10 +599,10 @@ read_srclines (Dwarf *dbg,
NEW_LINE (0);
/* Reset the flags. */
- basic_block = false;
- prologue_end = false;
- epilogue_begin = false;
- discriminator = 0;
+ state.basic_block = false;
+ state.prologue_end = false;
+ state.epilogue_begin = false;
+ state.discriminator = 0;
break;
case DW_LNS_advance_pc:
@@ -594,7 +626,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
get_sleb128 (s128, linep, lineendp);
- line += s128;
+ state.line += s128;
break;
case DW_LNS_set_file:
@@ -605,7 +637,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
get_uleb128 (u128, linep, lineendp);
- file = u128;
+ state.file = u128;
break;
case DW_LNS_set_column:
@@ -616,7 +648,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
get_uleb128 (u128, linep, lineendp);
- column = u128;
+ state.column = u128;
break;
case DW_LNS_negate_stmt:
@@ -624,7 +656,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- is_stmt = 1 - is_stmt;
+ state.is_stmt = 1 - state.is_stmt;
break;
case DW_LNS_set_basic_block:
@@ -632,7 +664,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- basic_block = true;
+ state.basic_block = true;
break;
case DW_LNS_const_add_pc:
@@ -653,8 +685,8 @@ read_srclines (Dwarf *dbg,
|| unlikely (lineendp - linep < 2))
goto invalid_data;
- addr += read_2ubyte_unaligned_inc (dbg, linep);
- op_index = 0;
+ state.addr += read_2ubyte_unaligned_inc (dbg, linep);
+ state.op_index = 0;
break;
case DW_LNS_set_prologue_end:
@@ -662,7 +694,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- prologue_end = true;
+ state.prologue_end = true;
break;
case DW_LNS_set_epilogue_begin:
@@ -670,7 +702,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- epilogue_begin = true;
+ state.epilogue_begin = true;
break;
case DW_LNS_set_isa:
@@ -680,7 +712,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
- get_uleb128 (isa, linep, lineendp);
+ get_uleb128 (state.isa, linep, lineendp);
break;
}
}
@@ -728,7 +760,8 @@ read_srclines (Dwarf *dbg,
if (filesp != NULL)
*filesp = files;
- size_t buf_size = (sizeof (Dwarf_Lines) + (sizeof (Dwarf_Line) * nlinelist));
+ size_t buf_size = (sizeof (Dwarf_Lines)
+ + (sizeof (Dwarf_Line) * state.nlinelist));
void *buf = libdw_alloc (dbg, Dwarf_Lines, buf_size, 1);
/* First use the buffer for the pointers, and sort the entries.
@@ -736,13 +769,13 @@ read_srclines (Dwarf *dbg,
copy into the buffer from the beginning so the overlap works. */
assert (sizeof (Dwarf_Line) >= sizeof (struct linelist *));
struct linelist **sortlines = (buf + buf_size
- - sizeof (struct linelist **) * nlinelist);
+ - sizeof (struct linelist **) * state.nlinelist);
/* The list is in LIFO order and usually they come in clumps with
ascending addresses. So fill from the back to probably start with
runs already in order before we sort. */
- struct linelist *lineslist = linelist;
- for (size_t i = nlinelist; i-- > 0; )
+ struct linelist *lineslist = state.linelist;
+ for (size_t i = state.nlinelist; i-- > 0; )
{
sortlines[i] = lineslist;
lineslist = lineslist->next;
@@ -750,14 +783,14 @@ read_srclines (Dwarf *dbg,
assert (lineslist == NULL);
/* Sort by ascending address. */
- qsort (sortlines, nlinelist, sizeof sortlines[0], &compare_lines);
+ qsort (sortlines, state.nlinelist, sizeof sortlines[0], &compare_lines);
/* Now that they are sorted, put them in the final array.
The buffers overlap, so we've clobbered the early elements
of SORTLINES by the time we're reading the later ones. */
Dwarf_Lines *lines = buf;
- lines->nlines = nlinelist;
- for (size_t i = 0; i < nlinelist; ++i)
+ lines->nlines = state.nlinelist;
+ for (size_t i = 0; i < state.nlinelist; ++i)
{
lines->info[i] = sortlines[i]->line;
lines->info[i].files = files;
@@ -766,8 +799,8 @@ read_srclines (Dwarf *dbg,
/* Make sure the highest address for the CU is marked as end_sequence.
This is required by the DWARF spec, but some compilers forget and
dwfl_module_getsrc depends on it. */
- if (nlinelist > 0)
- lines->info[nlinelist - 1].end_sequence = 1;
+ if (state.nlinelist > 0)
+ lines->info[state.nlinelist - 1].end_sequence = 1;
/* Pass the line structure back to the caller. */
if (linesp != NULL)
@@ -778,11 +811,11 @@ read_srclines (Dwarf *dbg,
out:
/* Free malloced line records, if any. */
- for (size_t i = MAX_STACK_LINES; i < nlinelist; i++)
+ for (size_t i = MAX_STACK_LINES; i < state.nlinelist; i++)
{
- struct linelist *ll = linelist->next;
- free (linelist);
- linelist = ll;
+ struct linelist *ll = state.linelist->next;
+ free (state.linelist);
+ state.linelist = ll;
}
if (ndirlist >= MAX_STACK_DIRS)
free (dirarray);
--
1.8.3.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] No nested functions in dwarf_get{srclines,scopevar}
@ 2015-11-13 17:12 Chih-Hung Hsieh
0 siblings, 0 replies; 9+ messages in thread
From: Chih-Hung Hsieh @ 2015-11-13 17:12 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 387 bytes --]
Thanks.
On Fri, Nov 13, 2015 at 9:10 AM, Mark Wielaard <mjw@redhat.com> wrote:
> On Thu, 2015-10-15 at 16:40 -0700, Chih-hung Hsieh wrote:
> > Please review the new attached 0002*patch file again.
>
> I pushed the dwarf_getscopevar part as attached. That one is small and
> seems correct. Still looking at the large changes in dwarf_getsrclines.
>
> Cheers,
>
> Mark
>
[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 718 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] No nested functions in dwarf_get{srclines,scopevar}
@ 2015-11-13 17:10 Mark Wielaard
0 siblings, 0 replies; 9+ messages in thread
From: Mark Wielaard @ 2015-11-13 17:10 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 282 bytes --]
On Thu, 2015-10-15 at 16:40 -0700, Chih-hung Hsieh wrote:
> Please review the new attached 0002*patch file again.
I pushed the dwarf_getscopevar part as attached. That one is small and
seems correct. Still looking at the large changes in dwarf_getsrclines.
Cheers,
Mark
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-No-nested-function-in-dwarf_getscopevar-to-file-scop.patch --]
[-- Type: text/x-patch, Size: 3204 bytes --]
From b926e56b7ae5fc31aee3d4bb8f40906c9aa001c2 Mon Sep 17 00:00:00 2001
From: Chih-Hung Hsieh <chh@google.com>
Date: Tue, 13 Oct 2015 15:26:14 -0700
Subject: [PATCH] No nested function in dwarf_getscopevar to file scope.
Signed-off-by: Chih-Hung Hsieh <chh@google.com>
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
libdw/ChangeLog | 5 +++++
libdw/dwarf_getscopevar.c | 39 ++++++++++++++++++++++-----------------
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 99ec2e8..fc044a2 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,8 @@
+2015-10-13 Chih-Hung Hsieh <chh@google.com>
+
+ * dwarf_getscopevar.c (dwarf_getscopevar): Move nested
+ function 'file_matches' to file scope.
+
2015-10-16 Mark Wielaard <mjw@redhat.com>
* Makefile.am (libdw.so): Add -lz.
diff --git a/libdw/dwarf_getscopevar.c b/libdw/dwarf_getscopevar.c
index eb50c0a..7b1416f 100644
--- a/libdw/dwarf_getscopevar.c
+++ b/libdw/dwarf_getscopevar.c
@@ -52,6 +52,26 @@ getattr (Dwarf_Die *die, int search_name, Dwarf_Word *value)
&attr_mem), value);
}
+static inline int
+file_matches (const char *lastfile,
+ size_t match_file_len, const char *match_file,
+ Dwarf_Files *files, size_t idx,
+ bool *lastfile_matches)
+{
+ if (idx >= files->nfiles)
+ return false;
+ const char *file = files->info[idx].name;
+ if (file != lastfile)
+ {
+ size_t len = strlen (file);
+ *lastfile_matches = (len >= match_file_len
+ && !memcmp (match_file, file, match_file_len)
+ && (len == match_file_len
+ || file[len - match_file_len - 1] == '/'));
+ }
+ return *lastfile_matches;
+}
+
/* Search SCOPES[0..NSCOPES-1] for a variable called NAME.
Ignore the first SKIP_SHADOWS scopes that match the name.
If MATCH_FILE is not null, accept only declaration in that source file;
@@ -72,22 +92,6 @@ dwarf_getscopevar (Dwarf_Die *scopes, int nscopes,
size_t match_file_len = match_file == NULL ? 0 : strlen (match_file);
bool lastfile_matches = false;
const char *lastfile = NULL;
- inline bool file_matches (Dwarf_Files *files, size_t idx)
- {
- if (idx >= files->nfiles)
- return false;
-
- const char *file = files->info[idx].name;
- if (file != lastfile)
- {
- size_t len = strlen (file);
- lastfile_matches = (len >= match_file_len
- && !memcmp (match_file, file, match_file_len)
- && (len == match_file_len
- || file[len - match_file_len - 1] == '/'));
- }
- return lastfile_matches;
- }
/* Start with the innermost scope and move out. */
for (int out = 0; out < nscopes; ++out)
@@ -130,7 +134,8 @@ dwarf_getscopevar (Dwarf_Die *scopes, int nscopes,
|| getfiles (&scopes[out], &files) != 0)
break;
- if (!file_matches (files, i))
+ if (!file_matches (lastfile, match_file_len, match_file,
+ files, i, &lastfile_matches))
break;
if (match_lineno > 0
--
1.8.3.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] No nested functions in dwarf_get{srclines,scopevar}
@ 2015-11-02 23:58 Chih-Hung Hsieh
0 siblings, 0 replies; 9+ messages in thread
From: Chih-Hung Hsieh @ 2015-11-02 23:58 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]
Mark, thanks for integrating a few of my previous patches to eliminate
nested functions.
I wonder if you have looked at this new 0002*patch and some other patches I
sent related to the move of nested functions. To help you find the other
patches, I will email you the list separately.
Thanks.
On Wed, Oct 21, 2015 at 4:04 PM, Chih-hung Hsieh <chh@google.com> wrote:
> Mark,
>
> The latest release 0.164 looked good to me. I have merged it into Android
> source tree.
> I am sending more patches to fix nested functions to compile with clang on
> Android.
> Changes for one or two files are put into a patch and tested separately
> so it should be okay to merge the patches in any order.
>
> I hope these many smaller patches are easier to merge for you.
> Please let me know if you prefer any different style.
> Thanks.
>
>
> On Wed, Oct 14, 2015 at 12:44 PM, Mark Wielaard <mjw@redhat.com> wrote:
>
>> On Tue, 2015-10-13 at 15:39 -0700, Chih-Hung Hsieh wrote:
>> > Move nested functions in libdw/dwarf_get{scopevar,srclines}.c
>> > to file scope to compile with clang/llvm.
>>
>> Since I would like to get a new release out on Friday and have a little
>> time to test things I am not immediately pushing these patches right
>> now. They are probably fine, but I like to look over them first. And
>> since they are somewhat large I am afraid we might introduce some subtle
>> bug right before the release. I'll get to them as soon as the release is
>> done on Friday.
>>
>
>
[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 2295 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] No nested functions in dwarf_get{srclines,scopevar}
@ 2015-10-21 23:04 Chih-Hung Hsieh
0 siblings, 0 replies; 9+ messages in thread
From: Chih-Hung Hsieh @ 2015-10-21 23:04 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]
Mark,
The latest release 0.164 looked good to me. I have merged it into Android
source tree.
I am sending more patches to fix nested functions to compile with clang on
Android.
Changes for one or two files are put into a patch and tested separately
so it should be okay to merge the patches in any order.
I hope these many smaller patches are easier to merge for you.
Please let me know if you prefer any different style.
Thanks.
On Wed, Oct 14, 2015 at 12:44 PM, Mark Wielaard <mjw@redhat.com> wrote:
> On Tue, 2015-10-13 at 15:39 -0700, Chih-Hung Hsieh wrote:
> > Move nested functions in libdw/dwarf_get{scopevar,srclines}.c
> > to file scope to compile with clang/llvm.
>
> Since I would like to get a new release out on Friday and have a little
> time to test things I am not immediately pushing these patches right
> now. They are probably fine, but I like to look over them first. And
> since they are somewhat large I am afraid we might introduce some subtle
> bug right before the release. I'll get to them as soon as the release is
> done on Friday.
>
[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1520 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] No nested functions in dwarf_get{srclines,scopevar}
@ 2015-10-15 23:40 Chih-Hung Hsieh
0 siblings, 0 replies; 9+ messages in thread
From: Chih-Hung Hsieh @ 2015-10-15 23:40 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 912 bytes --]
On Thu, Oct 15, 2015 at 1:18 PM, Roland McGrath <roland@hack.frob.com>
wrote:
> > +inline static int
>
> static inline int
Fixed.
>
> > +file_matches (Dwarf_Files * files, size_t idx,
>
> Gratuitous space after * there.
>
>
I found most other places do not have space after "*".
So I removed the extra space here to be consistent. :-)
> > + const char *lastfile, size_t match_file_len,
> > + const char *match_file, bool *lastfile_matches)
>
> When what should be a local function has several parameters like this to
> replace what should be the closed-over variables, I think it's clearer to
> put all those parameters first, before the real parameters to the function.
>
>
Changed, to have input read-only parameter first, followed by old
parameters and then output parameters.
Please review the new attached 0002*patch file again.
Thanks.
[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1699 bytes --]
[-- Attachment #3: 0002-No-nested-functions-in-dwarf_get-srclines-scopevar.patch --]
[-- Type: application/octet-stream, Size: 16884 bytes --]
From a0a0f38a86fa9d296469efa34e857f551f94ddd7 Mon Sep 17 00:00:00 2001
From: Chih-Hung Hsieh <chh@google.com>
Date: Tue, 13 Oct 2015 15:26:14 -0700
Subject: [PATCH] No nested functions in dwarf_get{srclines,scopevar}
Move nested functions in libdw/dwarf_get{scopevar,srclines}.c
to file scope to compile with clang/llvm.
Signed-off-by: Chih-Hung Hsieh <chh@google.com>
---
libdw/ChangeLog | 8 ++
libdw/dwarf_getscopevar.c | 39 ++++----
libdw/dwarf_getsrclines.c | 241 ++++++++++++++++++++++++++--------------------
3 files changed, 166 insertions(+), 122 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 39c49aa..837ccdf 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,11 @@
+2015-10-13 Chih-Hung Hsieh <chh@google.com>
+
+ * dwarf_getscopevar.c (dwarf_getscopevar): Move nested
+ function 'file_matches' to file scope.
+ * dwarf_getsrclines.c (read_srclines): Move nested functions
+ 'advance_pc' and 'add_new_line' to file scope and keep many
+ local state variables within one structure.
+
2015-10-09 Josh Stone <jistone@redhat.com>
* dwarf_begin.c (dwarf_begin): Replace stat64 and fstat64 with stat
diff --git a/libdw/dwarf_getscopevar.c b/libdw/dwarf_getscopevar.c
index eb50c0a..7b1416f 100644
--- a/libdw/dwarf_getscopevar.c
+++ b/libdw/dwarf_getscopevar.c
@@ -52,6 +52,26 @@ getattr (Dwarf_Die *die, int search_name, Dwarf_Word *value)
&attr_mem), value);
}
+static inline int
+file_matches (const char *lastfile,
+ size_t match_file_len, const char *match_file,
+ Dwarf_Files *files, size_t idx,
+ bool *lastfile_matches)
+{
+ if (idx >= files->nfiles)
+ return false;
+ const char *file = files->info[idx].name;
+ if (file != lastfile)
+ {
+ size_t len = strlen (file);
+ *lastfile_matches = (len >= match_file_len
+ && !memcmp (match_file, file, match_file_len)
+ && (len == match_file_len
+ || file[len - match_file_len - 1] == '/'));
+ }
+ return *lastfile_matches;
+}
+
/* Search SCOPES[0..NSCOPES-1] for a variable called NAME.
Ignore the first SKIP_SHADOWS scopes that match the name.
If MATCH_FILE is not null, accept only declaration in that source file;
@@ -72,22 +92,6 @@ dwarf_getscopevar (Dwarf_Die *scopes, int nscopes,
size_t match_file_len = match_file == NULL ? 0 : strlen (match_file);
bool lastfile_matches = false;
const char *lastfile = NULL;
- inline bool file_matches (Dwarf_Files *files, size_t idx)
- {
- if (idx >= files->nfiles)
- return false;
-
- const char *file = files->info[idx].name;
- if (file != lastfile)
- {
- size_t len = strlen (file);
- lastfile_matches = (len >= match_file_len
- && !memcmp (match_file, file, match_file_len)
- && (len == match_file_len
- || file[len - match_file_len - 1] == '/'));
- }
- return lastfile_matches;
- }
/* Start with the innermost scope and move out. */
for (int out = 0; out < nscopes; ++out)
@@ -130,7 +134,8 @@ dwarf_getscopevar (Dwarf_Die *scopes, int nscopes,
|| getfiles (&scopes[out], &files) != 0)
break;
- if (!file_matches (files, i))
+ if (!file_matches (lastfile, match_file_len, match_file,
+ files, i, &lastfile_matches))
break;
if (match_lineno > 0
diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c
index 389c824..a8b57e1 100644
--- a/libdw/dwarf_getsrclines.c
+++ b/libdw/dwarf_getsrclines.c
@@ -78,6 +78,73 @@ compare_lines (const void *a, const void *b)
: 0;
}
+struct line_state {
+ Dwarf_Word addr;
+ unsigned int op_index;
+ unsigned int file;
+ int64_t line;
+ unsigned int column;
+ uint_fast8_t is_stmt;
+ bool basic_block;
+ bool prologue_end;
+ bool epilogue_begin;
+ unsigned int isa;
+ unsigned int discriminator;
+ struct linelist *linelist;
+ size_t nlinelist;
+ unsigned int end_sequence;
+};
+
+static inline void
+run_advance_pc (struct line_state *state, unsigned int op_advance,
+ uint_fast8_t minimum_instr_len, uint_fast8_t max_ops_per_instr)
+{
+ state->addr += minimum_instr_len * ((state->op_index + op_advance)
+ / max_ops_per_instr);
+ state->op_index = (state->op_index + op_advance) % max_ops_per_instr;
+}
+
+static inline bool
+add_new_line (struct line_state *state, struct linelist *new_line)
+{
+ /* Set the line information. For some fields we use bitfields,
+ so we would lose information if the encoded values are too large.
+ Check just for paranoia, and call the data "invalid" if it
+ violates our assumptions on reasonable limits for the values. */
+ new_line->next = state->linelist;
+ new_line->sequence = state->nlinelist;
+ state->linelist = new_line;
+ ++(state->nlinelist);
+
+ /* Set the line information. For some fields we use bitfields,
+ so we would lose information if the encoded values are too large.
+ Check just for paranoia, and call the data "invalid" if it
+ violates our assumptions on reasonable limits for the values. */
+#define SET(field) \
+ do { \
+ new_line->line.field = state->field; \
+ if (unlikely (new_line->line.field != state->field)) \
+ return true; \
+ } while (0)
+
+ SET (addr);
+ SET (op_index);
+ SET (file);
+ SET (line);
+ SET (column);
+ SET (is_stmt);
+ SET (basic_block);
+ SET (end_sequence);
+ SET (prologue_end);
+ SET (epilogue_begin);
+ SET (isa);
+ SET (discriminator);
+
+#undef SET
+
+ return false;
+}
+
static int
read_srclines (Dwarf *dbg,
const unsigned char *linep, const unsigned char *lineendp,
@@ -86,8 +153,6 @@ read_srclines (Dwarf *dbg,
{
int res = -1;
- struct linelist *linelist = NULL;
- size_t nlinelist = 0;
size_t nfilelist = 0;
unsigned int ndirlist = 0;
@@ -323,27 +388,28 @@ read_srclines (Dwarf *dbg,
/* We are about to process the statement program. Initialize the
state machine registers (see 6.2.2 in the v2.1 specification). */
- Dwarf_Word addr = 0;
- unsigned int op_index = 0;
- unsigned int file = 1;
- /* We only store an int, but want to check for overflow (see SET below). */
- int64_t line = 1;
- unsigned int column = 0;
- uint_fast8_t is_stmt = default_is_stmt;
- bool basic_block = false;
- bool prologue_end = false;
- bool epilogue_begin = false;
- unsigned int isa = 0;
- unsigned int discriminator = 0;
+ struct line_state state =
+ {
+ .linelist = NULL,
+ .nlinelist = 0,
+ .addr = 0,
+ .op_index = 0,
+ .file = 1,
+ /* We only store an int, but want to check for overflow (see SET below). */
+ .line = 1,
+ .column = 0,
+ .is_stmt = default_is_stmt,
+ .basic_block = false,
+ .prologue_end = false,
+ .epilogue_begin = false,
+ .isa = 0,
+ .discriminator = 0
+ };
/* Apply the "operation advance" from a special opcode or
DW_LNS_advance_pc (as per DWARF4 6.2.5.1). */
- inline void advance_pc (unsigned int op_advance)
- {
- addr += minimum_instr_len * ((op_index + op_advance)
- / max_ops_per_instr);
- op_index = (op_index + op_advance) % max_ops_per_instr;
- }
+#define advance_pc(op_advance) \
+ run_advance_pc (&state, op_advance, minimum_instr_len, max_ops_per_instr)
/* Process the instructions. */
@@ -352,51 +418,16 @@ read_srclines (Dwarf *dbg,
struct linelist llstack[MAX_STACK_LINES];
#define NEW_LINE(end_seq) \
do { \
- struct linelist *ll = (nlinelist < MAX_STACK_LINES \
- ? &llstack[nlinelist] \
+ struct linelist *ll = (state.nlinelist < MAX_STACK_LINES \
+ ? &llstack[state.nlinelist] \
: malloc (sizeof (struct linelist))); \
if (unlikely (ll == NULL)) \
goto no_mem; \
- if (unlikely (add_new_line (ll, end_seq))) \
+ state.end_sequence = end_seq; \
+ if (unlikely (add_new_line (&state, ll))) \
goto invalid_data; \
} while (0)
- inline bool add_new_line (struct linelist *new_line, bool end_sequence)
- {
- new_line->next = linelist;
- new_line->sequence = nlinelist;
- linelist = new_line;
- ++nlinelist;
-
- /* Set the line information. For some fields we use bitfields,
- so we would lose information if the encoded values are too large.
- Check just for paranoia, and call the data "invalid" if it
- violates our assumptions on reasonable limits for the values. */
-#define SET(field) \
- do { \
- new_line->line.field = field; \
- if (unlikely (new_line->line.field != field)) \
- return true; \
- } while (0)
-
- SET (addr);
- SET (op_index);
- SET (file);
- SET (line);
- SET (column);
- SET (is_stmt);
- SET (basic_block);
- SET (end_sequence);
- SET (prologue_end);
- SET (epilogue_begin);
- SET (isa);
- SET (discriminator);
-
-#undef SET
-
- return false;
- }
-
while (linep < lineendp)
{
unsigned int opcode;
@@ -422,17 +453,17 @@ read_srclines (Dwarf *dbg,
+ (opcode - opcode_base) % line_range);
/* Perform the increments. */
- line += line_increment;
+ state.line += line_increment;
advance_pc ((opcode - opcode_base) / line_range);
/* Add a new line with the current state machine values. */
NEW_LINE (0);
/* Reset the flags. */
- basic_block = false;
- prologue_end = false;
- epilogue_begin = false;
- discriminator = 0;
+ state.basic_block = false;
+ state.prologue_end = false;
+ state.epilogue_begin = false;
+ state.discriminator = 0;
}
else if (opcode == 0)
{
@@ -457,28 +488,28 @@ read_srclines (Dwarf *dbg,
NEW_LINE (1);
/* Reset the registers. */
- addr = 0;
- op_index = 0;
- file = 1;
- line = 1;
- column = 0;
- is_stmt = default_is_stmt;
- basic_block = false;
- prologue_end = false;
- epilogue_begin = false;
- isa = 0;
- discriminator = 0;
+ state.addr = 0;
+ state.op_index = 0;
+ state.file = 1;
+ state.line = 1;
+ state.column = 0;
+ state.is_stmt = default_is_stmt;
+ state.basic_block = false;
+ state.prologue_end = false;
+ state.epilogue_begin = false;
+ state.isa = 0;
+ state.discriminator = 0;
break;
case DW_LNE_set_address:
/* The value is an address. The size is defined as
apporiate for the target machine. We use the
address size field from the CU header. */
- op_index = 0;
+ state.op_index = 0;
if (unlikely (lineendp - linep < (uint8_t) address_size))
goto invalid_data;
if (__libdw_read_address_inc (dbg, IDX_debug_line, &linep,
- address_size, &addr))
+ address_size, &state.addr))
goto out;
break;
@@ -542,7 +573,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
- get_uleb128 (discriminator, linep, lineendp);
+ get_uleb128 (state.discriminator, linep, lineendp);
break;
default:
@@ -567,10 +598,10 @@ read_srclines (Dwarf *dbg,
NEW_LINE (0);
/* Reset the flags. */
- basic_block = false;
- prologue_end = false;
- epilogue_begin = false;
- discriminator = 0;
+ state.basic_block = false;
+ state.prologue_end = false;
+ state.epilogue_begin = false;
+ state.discriminator = 0;
break;
case DW_LNS_advance_pc:
@@ -594,7 +625,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
get_sleb128 (s128, linep, lineendp);
- line += s128;
+ state.line += s128;
break;
case DW_LNS_set_file:
@@ -605,7 +636,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
get_uleb128 (u128, linep, lineendp);
- file = u128;
+ state.file = u128;
break;
case DW_LNS_set_column:
@@ -616,7 +647,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
get_uleb128 (u128, linep, lineendp);
- column = u128;
+ state.column = u128;
break;
case DW_LNS_negate_stmt:
@@ -624,7 +655,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- is_stmt = 1 - is_stmt;
+ state.is_stmt = 1 - state.is_stmt;
break;
case DW_LNS_set_basic_block:
@@ -632,7 +663,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- basic_block = true;
+ state.basic_block = true;
break;
case DW_LNS_const_add_pc:
@@ -653,8 +684,8 @@ read_srclines (Dwarf *dbg,
|| unlikely (lineendp - linep < 2))
goto invalid_data;
- addr += read_2ubyte_unaligned_inc (dbg, linep);
- op_index = 0;
+ state.addr += read_2ubyte_unaligned_inc (dbg, linep);
+ state.op_index = 0;
break;
case DW_LNS_set_prologue_end:
@@ -662,7 +693,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- prologue_end = true;
+ state.prologue_end = true;
break;
case DW_LNS_set_epilogue_begin:
@@ -670,7 +701,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- epilogue_begin = true;
+ state.epilogue_begin = true;
break;
case DW_LNS_set_isa:
@@ -680,7 +711,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
- get_uleb128 (isa, linep, lineendp);
+ get_uleb128 (state.isa, linep, lineendp);
break;
}
}
@@ -728,7 +759,7 @@ read_srclines (Dwarf *dbg,
if (filesp != NULL)
*filesp = files;
- size_t buf_size = (sizeof (Dwarf_Lines) + (sizeof (Dwarf_Line) * nlinelist));
+ size_t buf_size = (sizeof (Dwarf_Lines) + (sizeof (Dwarf_Line) * state.nlinelist));
void *buf = libdw_alloc (dbg, Dwarf_Lines, buf_size, 1);
/* First use the buffer for the pointers, and sort the entries.
@@ -736,13 +767,13 @@ read_srclines (Dwarf *dbg,
copy into the buffer from the beginning so the overlap works. */
assert (sizeof (Dwarf_Line) >= sizeof (struct linelist *));
struct linelist **sortlines = (buf + buf_size
- - sizeof (struct linelist **) * nlinelist);
+ - sizeof (struct linelist **) * state.nlinelist);
/* The list is in LIFO order and usually they come in clumps with
ascending addresses. So fill from the back to probably start with
runs already in order before we sort. */
- struct linelist *lineslist = linelist;
- for (size_t i = nlinelist; i-- > 0; )
+ struct linelist *lineslist = state.linelist;
+ for (size_t i = state.nlinelist; i-- > 0; )
{
sortlines[i] = lineslist;
lineslist = lineslist->next;
@@ -750,14 +781,14 @@ read_srclines (Dwarf *dbg,
assert (lineslist == NULL);
/* Sort by ascending address. */
- qsort (sortlines, nlinelist, sizeof sortlines[0], &compare_lines);
+ qsort (sortlines, state.nlinelist, sizeof sortlines[0], &compare_lines);
/* Now that they are sorted, put them in the final array.
The buffers overlap, so we've clobbered the early elements
of SORTLINES by the time we're reading the later ones. */
Dwarf_Lines *lines = buf;
- lines->nlines = nlinelist;
- for (size_t i = 0; i < nlinelist; ++i)
+ lines->nlines = state.nlinelist;
+ for (size_t i = 0; i < state.nlinelist; ++i)
{
lines->info[i] = sortlines[i]->line;
lines->info[i].files = files;
@@ -766,8 +797,8 @@ read_srclines (Dwarf *dbg,
/* Make sure the highest address for the CU is marked as end_sequence.
This is required by the DWARF spec, but some compilers forget and
dwfl_module_getsrc depends on it. */
- if (nlinelist > 0)
- lines->info[nlinelist - 1].end_sequence = 1;
+ if (state.nlinelist > 0)
+ lines->info[state.nlinelist - 1].end_sequence = 1;
/* Pass the line structure back to the caller. */
if (linesp != NULL)
@@ -778,11 +809,11 @@ read_srclines (Dwarf *dbg,
out:
/* Free malloced line records, if any. */
- for (size_t i = MAX_STACK_LINES; i < nlinelist; i++)
+ for (size_t i = MAX_STACK_LINES; i < state.nlinelist; i++)
{
- struct linelist *ll = linelist->next;
- free (linelist);
- linelist = ll;
+ struct linelist *ll = state.linelist->next;
+ free (state.linelist);
+ state.linelist = ll;
}
if (ndirlist >= MAX_STACK_DIRS)
free (dirarray);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] No nested functions in dwarf_get{srclines,scopevar}
@ 2015-10-15 20:18 Roland McGrath
0 siblings, 0 replies; 9+ messages in thread
From: Roland McGrath @ 2015-10-15 20:18 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 492 bytes --]
> +inline static int
static inline int
> +file_matches (Dwarf_Files * files, size_t idx,
Gratuitous space after * there.
> + const char *lastfile, size_t match_file_len,
> + const char *match_file, bool *lastfile_matches)
When what should be a local function has several parameters like this to
replace what should be the closed-over variables, I think it's clearer to
put all those parameters first, before the real parameters to the function.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] No nested functions in dwarf_get{srclines,scopevar}
@ 2015-10-13 22:39 Chih-Hung Hsieh
0 siblings, 0 replies; 9+ messages in thread
From: Chih-Hung Hsieh @ 2015-10-13 22:39 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 17177 bytes --]
Move nested functions in libdw/dwarf_get{scopevar,srclines}.c
to file scope to compile with clang/llvm.
Signed-off-by: Chih-Hung Hsieh <chh@google.com>
---
libdw/ChangeLog | 8 ++
libdw/dwarf_getscopevar.c | 38 ++++----
libdw/dwarf_getsrclines.c | 241 ++++++++++++++++++++++++++--------------------
3 files changed, 165 insertions(+), 122 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 39c49aa..837ccdf 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,11 @@
+2015-10-13 Chih-Hung Hsieh <chh@google.com>
+
+ * dwarf_getscopevar.c (dwarf_getscopevar): Move nested
+ function 'file_matches' to file scope.
+ * dwarf_getsrclines.c (read_srclines): Move nested functions
+ 'advance_pc' and 'add_new_line' to file scope and keep many
+ local state variables within one structure.
+
2015-10-09 Josh Stone <jistone@redhat.com>
* dwarf_begin.c (dwarf_begin): Replace stat64 and fstat64 with stat
diff --git a/libdw/dwarf_getscopevar.c b/libdw/dwarf_getscopevar.c
index eb50c0a..e59dccd 100644
--- a/libdw/dwarf_getscopevar.c
+++ b/libdw/dwarf_getscopevar.c
@@ -52,6 +52,25 @@ getattr (Dwarf_Die *die, int search_name, Dwarf_Word *value)
&attr_mem), value);
}
+inline static int
+file_matches (Dwarf_Files * files, size_t idx,
+ const char *lastfile, size_t match_file_len,
+ const char *match_file, bool *lastfile_matches)
+{
+ if (idx >= files->nfiles)
+ return false;
+ const char *file = files->info[idx].name;
+ if (file != lastfile)
+ {
+ size_t len = strlen (file);
+ *lastfile_matches = (len >= match_file_len
+ && !memcmp (match_file, file, match_file_len)
+ && (len == match_file_len
+ || file[len - match_file_len - 1] == '/'));
+ }
+ return *lastfile_matches;
+}
+
/* Search SCOPES[0..NSCOPES-1] for a variable called NAME.
Ignore the first SKIP_SHADOWS scopes that match the name.
If MATCH_FILE is not null, accept only declaration in that source file;
@@ -72,22 +91,6 @@ dwarf_getscopevar (Dwarf_Die *scopes, int nscopes,
size_t match_file_len = match_file == NULL ? 0 : strlen (match_file);
bool lastfile_matches = false;
const char *lastfile = NULL;
- inline bool file_matches (Dwarf_Files *files, size_t idx)
- {
- if (idx >= files->nfiles)
- return false;
-
- const char *file = files->info[idx].name;
- if (file != lastfile)
- {
- size_t len = strlen (file);
- lastfile_matches = (len >= match_file_len
- && !memcmp (match_file, file, match_file_len)
- && (len == match_file_len
- || file[len - match_file_len - 1] == '/'));
- }
- return lastfile_matches;
- }
/* Start with the innermost scope and move out. */
for (int out = 0; out < nscopes; ++out)
@@ -130,7 +133,8 @@ dwarf_getscopevar (Dwarf_Die *scopes, int nscopes,
|| getfiles (&scopes[out], &files) != 0)
break;
- if (!file_matches (files, i))
+ if (!file_matches (files, i, lastfile, match_file_len,
+ match_file, &lastfile_matches))
break;
if (match_lineno > 0
diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c
index 389c824..a8b57e1 100644
--- a/libdw/dwarf_getsrclines.c
+++ b/libdw/dwarf_getsrclines.c
@@ -78,6 +78,73 @@ compare_lines (const void *a, const void *b)
: 0;
}
+struct line_state {
+ Dwarf_Word addr;
+ unsigned int op_index;
+ unsigned int file;
+ int64_t line;
+ unsigned int column;
+ uint_fast8_t is_stmt;
+ bool basic_block;
+ bool prologue_end;
+ bool epilogue_begin;
+ unsigned int isa;
+ unsigned int discriminator;
+ struct linelist *linelist;
+ size_t nlinelist;
+ unsigned int end_sequence;
+};
+
+static inline void
+run_advance_pc (struct line_state *state, unsigned int op_advance,
+ uint_fast8_t minimum_instr_len, uint_fast8_t max_ops_per_instr)
+{
+ state->addr += minimum_instr_len * ((state->op_index + op_advance)
+ / max_ops_per_instr);
+ state->op_index = (state->op_index + op_advance) % max_ops_per_instr;
+}
+
+static inline bool
+add_new_line (struct line_state *state, struct linelist *new_line)
+{
+ /* Set the line information. For some fields we use bitfields,
+ so we would lose information if the encoded values are too large.
+ Check just for paranoia, and call the data "invalid" if it
+ violates our assumptions on reasonable limits for the values. */
+ new_line->next = state->linelist;
+ new_line->sequence = state->nlinelist;
+ state->linelist = new_line;
+ ++(state->nlinelist);
+
+ /* Set the line information. For some fields we use bitfields,
+ so we would lose information if the encoded values are too large.
+ Check just for paranoia, and call the data "invalid" if it
+ violates our assumptions on reasonable limits for the values. */
+#define SET(field) \
+ do { \
+ new_line->line.field = state->field; \
+ if (unlikely (new_line->line.field != state->field)) \
+ return true; \
+ } while (0)
+
+ SET (addr);
+ SET (op_index);
+ SET (file);
+ SET (line);
+ SET (column);
+ SET (is_stmt);
+ SET (basic_block);
+ SET (end_sequence);
+ SET (prologue_end);
+ SET (epilogue_begin);
+ SET (isa);
+ SET (discriminator);
+
+#undef SET
+
+ return false;
+}
+
static int
read_srclines (Dwarf *dbg,
const unsigned char *linep, const unsigned char *lineendp,
@@ -86,8 +153,6 @@ read_srclines (Dwarf *dbg,
{
int res = -1;
- struct linelist *linelist = NULL;
- size_t nlinelist = 0;
size_t nfilelist = 0;
unsigned int ndirlist = 0;
@@ -323,27 +388,28 @@ read_srclines (Dwarf *dbg,
/* We are about to process the statement program. Initialize the
state machine registers (see 6.2.2 in the v2.1 specification). */
- Dwarf_Word addr = 0;
- unsigned int op_index = 0;
- unsigned int file = 1;
- /* We only store an int, but want to check for overflow (see SET below). */
- int64_t line = 1;
- unsigned int column = 0;
- uint_fast8_t is_stmt = default_is_stmt;
- bool basic_block = false;
- bool prologue_end = false;
- bool epilogue_begin = false;
- unsigned int isa = 0;
- unsigned int discriminator = 0;
+ struct line_state state =
+ {
+ .linelist = NULL,
+ .nlinelist = 0,
+ .addr = 0,
+ .op_index = 0,
+ .file = 1,
+ /* We only store an int, but want to check for overflow (see SET below). */
+ .line = 1,
+ .column = 0,
+ .is_stmt = default_is_stmt,
+ .basic_block = false,
+ .prologue_end = false,
+ .epilogue_begin = false,
+ .isa = 0,
+ .discriminator = 0
+ };
/* Apply the "operation advance" from a special opcode or
DW_LNS_advance_pc (as per DWARF4 6.2.5.1). */
- inline void advance_pc (unsigned int op_advance)
- {
- addr += minimum_instr_len * ((op_index + op_advance)
- / max_ops_per_instr);
- op_index = (op_index + op_advance) % max_ops_per_instr;
- }
+#define advance_pc(op_advance) \
+ run_advance_pc (&state, op_advance, minimum_instr_len, max_ops_per_instr)
/* Process the instructions. */
@@ -352,51 +418,16 @@ read_srclines (Dwarf *dbg,
struct linelist llstack[MAX_STACK_LINES];
#define NEW_LINE(end_seq) \
do { \
- struct linelist *ll = (nlinelist < MAX_STACK_LINES \
- ? &llstack[nlinelist] \
+ struct linelist *ll = (state.nlinelist < MAX_STACK_LINES \
+ ? &llstack[state.nlinelist] \
: malloc (sizeof (struct linelist))); \
if (unlikely (ll == NULL)) \
goto no_mem; \
- if (unlikely (add_new_line (ll, end_seq))) \
+ state.end_sequence = end_seq; \
+ if (unlikely (add_new_line (&state, ll))) \
goto invalid_data; \
} while (0)
- inline bool add_new_line (struct linelist *new_line, bool end_sequence)
- {
- new_line->next = linelist;
- new_line->sequence = nlinelist;
- linelist = new_line;
- ++nlinelist;
-
- /* Set the line information. For some fields we use bitfields,
- so we would lose information if the encoded values are too large.
- Check just for paranoia, and call the data "invalid" if it
- violates our assumptions on reasonable limits for the values. */
-#define SET(field) \
- do { \
- new_line->line.field = field; \
- if (unlikely (new_line->line.field != field)) \
- return true; \
- } while (0)
-
- SET (addr);
- SET (op_index);
- SET (file);
- SET (line);
- SET (column);
- SET (is_stmt);
- SET (basic_block);
- SET (end_sequence);
- SET (prologue_end);
- SET (epilogue_begin);
- SET (isa);
- SET (discriminator);
-
-#undef SET
-
- return false;
- }
-
while (linep < lineendp)
{
unsigned int opcode;
@@ -422,17 +453,17 @@ read_srclines (Dwarf *dbg,
+ (opcode - opcode_base) % line_range);
/* Perform the increments. */
- line += line_increment;
+ state.line += line_increment;
advance_pc ((opcode - opcode_base) / line_range);
/* Add a new line with the current state machine values. */
NEW_LINE (0);
/* Reset the flags. */
- basic_block = false;
- prologue_end = false;
- epilogue_begin = false;
- discriminator = 0;
+ state.basic_block = false;
+ state.prologue_end = false;
+ state.epilogue_begin = false;
+ state.discriminator = 0;
}
else if (opcode == 0)
{
@@ -457,28 +488,28 @@ read_srclines (Dwarf *dbg,
NEW_LINE (1);
/* Reset the registers. */
- addr = 0;
- op_index = 0;
- file = 1;
- line = 1;
- column = 0;
- is_stmt = default_is_stmt;
- basic_block = false;
- prologue_end = false;
- epilogue_begin = false;
- isa = 0;
- discriminator = 0;
+ state.addr = 0;
+ state.op_index = 0;
+ state.file = 1;
+ state.line = 1;
+ state.column = 0;
+ state.is_stmt = default_is_stmt;
+ state.basic_block = false;
+ state.prologue_end = false;
+ state.epilogue_begin = false;
+ state.isa = 0;
+ state.discriminator = 0;
break;
case DW_LNE_set_address:
/* The value is an address. The size is defined as
apporiate for the target machine. We use the
address size field from the CU header. */
- op_index = 0;
+ state.op_index = 0;
if (unlikely (lineendp - linep < (uint8_t) address_size))
goto invalid_data;
if (__libdw_read_address_inc (dbg, IDX_debug_line, &linep,
- address_size, &addr))
+ address_size, &state.addr))
goto out;
break;
@@ -542,7 +573,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
- get_uleb128 (discriminator, linep, lineendp);
+ get_uleb128 (state.discriminator, linep, lineendp);
break;
default:
@@ -567,10 +598,10 @@ read_srclines (Dwarf *dbg,
NEW_LINE (0);
/* Reset the flags. */
- basic_block = false;
- prologue_end = false;
- epilogue_begin = false;
- discriminator = 0;
+ state.basic_block = false;
+ state.prologue_end = false;
+ state.epilogue_begin = false;
+ state.discriminator = 0;
break;
case DW_LNS_advance_pc:
@@ -594,7 +625,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
get_sleb128 (s128, linep, lineendp);
- line += s128;
+ state.line += s128;
break;
case DW_LNS_set_file:
@@ -605,7 +636,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
get_uleb128 (u128, linep, lineendp);
- file = u128;
+ state.file = u128;
break;
case DW_LNS_set_column:
@@ -616,7 +647,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
get_uleb128 (u128, linep, lineendp);
- column = u128;
+ state.column = u128;
break;
case DW_LNS_negate_stmt:
@@ -624,7 +655,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- is_stmt = 1 - is_stmt;
+ state.is_stmt = 1 - state.is_stmt;
break;
case DW_LNS_set_basic_block:
@@ -632,7 +663,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- basic_block = true;
+ state.basic_block = true;
break;
case DW_LNS_const_add_pc:
@@ -653,8 +684,8 @@ read_srclines (Dwarf *dbg,
|| unlikely (lineendp - linep < 2))
goto invalid_data;
- addr += read_2ubyte_unaligned_inc (dbg, linep);
- op_index = 0;
+ state.addr += read_2ubyte_unaligned_inc (dbg, linep);
+ state.op_index = 0;
break;
case DW_LNS_set_prologue_end:
@@ -662,7 +693,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- prologue_end = true;
+ state.prologue_end = true;
break;
case DW_LNS_set_epilogue_begin:
@@ -670,7 +701,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (standard_opcode_lengths[opcode] != 0))
goto invalid_data;
- epilogue_begin = true;
+ state.epilogue_begin = true;
break;
case DW_LNS_set_isa:
@@ -680,7 +711,7 @@ read_srclines (Dwarf *dbg,
if (unlikely (linep >= lineendp))
goto invalid_data;
- get_uleb128 (isa, linep, lineendp);
+ get_uleb128 (state.isa, linep, lineendp);
break;
}
}
@@ -728,7 +759,7 @@ read_srclines (Dwarf *dbg,
if (filesp != NULL)
*filesp = files;
- size_t buf_size = (sizeof (Dwarf_Lines) + (sizeof (Dwarf_Line) * nlinelist));
+ size_t buf_size = (sizeof (Dwarf_Lines) + (sizeof (Dwarf_Line) * state.nlinelist));
void *buf = libdw_alloc (dbg, Dwarf_Lines, buf_size, 1);
/* First use the buffer for the pointers, and sort the entries.
@@ -736,13 +767,13 @@ read_srclines (Dwarf *dbg,
copy into the buffer from the beginning so the overlap works. */
assert (sizeof (Dwarf_Line) >= sizeof (struct linelist *));
struct linelist **sortlines = (buf + buf_size
- - sizeof (struct linelist **) * nlinelist);
+ - sizeof (struct linelist **) * state.nlinelist);
/* The list is in LIFO order and usually they come in clumps with
ascending addresses. So fill from the back to probably start with
runs already in order before we sort. */
- struct linelist *lineslist = linelist;
- for (size_t i = nlinelist; i-- > 0; )
+ struct linelist *lineslist = state.linelist;
+ for (size_t i = state.nlinelist; i-- > 0; )
{
sortlines[i] = lineslist;
lineslist = lineslist->next;
@@ -750,14 +781,14 @@ read_srclines (Dwarf *dbg,
assert (lineslist == NULL);
/* Sort by ascending address. */
- qsort (sortlines, nlinelist, sizeof sortlines[0], &compare_lines);
+ qsort (sortlines, state.nlinelist, sizeof sortlines[0], &compare_lines);
/* Now that they are sorted, put them in the final array.
The buffers overlap, so we've clobbered the early elements
of SORTLINES by the time we're reading the later ones. */
Dwarf_Lines *lines = buf;
- lines->nlines = nlinelist;
- for (size_t i = 0; i < nlinelist; ++i)
+ lines->nlines = state.nlinelist;
+ for (size_t i = 0; i < state.nlinelist; ++i)
{
lines->info[i] = sortlines[i]->line;
lines->info[i].files = files;
@@ -766,8 +797,8 @@ read_srclines (Dwarf *dbg,
/* Make sure the highest address for the CU is marked as end_sequence.
This is required by the DWARF spec, but some compilers forget and
dwfl_module_getsrc depends on it. */
- if (nlinelist > 0)
- lines->info[nlinelist - 1].end_sequence = 1;
+ if (state.nlinelist > 0)
+ lines->info[state.nlinelist - 1].end_sequence = 1;
/* Pass the line structure back to the caller. */
if (linesp != NULL)
@@ -778,11 +809,11 @@ read_srclines (Dwarf *dbg,
out:
/* Free malloced line records, if any. */
- for (size_t i = MAX_STACK_LINES; i < nlinelist; i++)
+ for (size_t i = MAX_STACK_LINES; i < state.nlinelist; i++)
{
- struct linelist *ll = linelist->next;
- free (linelist);
- linelist = ll;
+ struct linelist *ll = state.linelist->next;
+ free (state.linelist);
+ state.linelist = ll;
}
if (ndirlist >= MAX_STACK_DIRS)
free (dirarray);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-11-13 21:21 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-14 19:44 [PATCH] No nested functions in dwarf_get{srclines,scopevar} Mark Wielaard
-- strict thread matches above, loose matches on Subject: below --
2015-11-13 21:21 Mark Wielaard
2015-11-13 17:12 Chih-Hung Hsieh
2015-11-13 17:10 Mark Wielaard
2015-11-02 23:58 Chih-Hung Hsieh
2015-10-21 23:04 Chih-Hung Hsieh
2015-10-15 23:40 Chih-Hung Hsieh
2015-10-15 20:18 Roland McGrath
2015-10-13 22:39 Chih-Hung Hsieh
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).