[PATCH 1/3] Pull a couple common AWK functions for CFI scripts into separate file

[PATCH 1/3] Pull a couple common AWK functions for CFI scripts into separate file

[Alex Dowad]

There is a lot which could be common between i386 and x86_64, but none of it
will be useful for any other arch. These should be useful for all archs,
however.
---
 Makefile               |  2 +-
 tools/add-cfi.i386.awk | 27 ---------------------------
 tools/common.awk       | 26 ++++++++++++++++++++++++++
 3 files changed, 27 insertions(+), 28 deletions(-)
 create mode 100644 tools/common.awk

diff --git a/Makefile b/Makefile
index 5a6a43b..761482d 100644
--- a/Makefile
+++ b/Makefile
@@ -122,7 +122,7 @@ $(foreach s,$(wildcard src/*/$(ARCH)*/*.s),$(eval $(call mkasmdep,$(s))))
 # Choose invocation of assembler to be used
 # $(1) is input file, $(2) is output file, $(3) is assembler flags
 ifeq ($(ADD_CFI),yes)
-	AS_CMD = LC_ALL=C awk -f tools/add-cfi.$(ARCH).awk $< | $(CC) -x assembler -c -o $@ -
+	AS_CMD = LC_ALL=C awk -f tools/common.awk -f tools/add-cfi.$(ARCH).awk $< | $(CC) -x assembler -c -o $@ -
 else
 	AS_CMD = $(CC) -c -o $@ $<
 endif
diff --git a/tools/add-cfi.i386.awk b/tools/add-cfi.i386.awk
index 4a4a3b6..b8bdd7f 100644
--- a/tools/add-cfi.i386.awk
+++ b/tools/add-cfi.i386.awk
@@ -22,33 +22,6 @@ BEGIN {
   called = ""
 }
 
-function hex2int(str,   i) {
-  str = tolower(str)
-
-  for (i = 1; i <= 16; i++) {
-    char = substr("0123456789abcdef", i, 1)
-    lookup[char] = i-1
-  }
-
-  result = 0
-  for (i = 1; i <= length(str); i++) {
-    result = result * 16
-    char   = substr(str, i, 1)
-    result = result + lookup[char]
-  }
-  return result
-}
-
-function parse_const(str) {
-  sign = sub(/^-/, "", str)
-  hex  = sub(/^0x/, "", str)
-  if (hex)
-    n = hex2int(str)
-  else
-    n = str+0
-  return sign ? -n : n
-}
-
 function get_const1() {
   # for instructions with 2 operands, get 1st operand (assuming it is constant)
   match($0, /-?(0x[0-9a-fA-F]+|[0-9]+),/)
diff --git a/tools/common.awk b/tools/common.awk
new file mode 100644
index 0000000..04482d4
--- /dev/null
+++ b/tools/common.awk
@@ -0,0 +1,26 @@
+function hex2int(str,   i) {
+  str = tolower(str)
+
+  for (i = 1; i <= 16; i++) {
+    char = substr("0123456789abcdef", i, 1)
+    lookup[char] = i-1
+  }
+
+  result = 0
+  for (i = 1; i <= length(str); i++) {
+    result = result * 16
+    char   = substr(str, i, 1)
+    result = result + lookup[char]
+  }
+  return result
+}
+
+function parse_const(str) {
+  sign = sub(/^-/, "", str)
+  hex  = sub(/^0x/, "", str)
+  if (hex)
+    n = hex2int(str)
+  else
+    n = str+0
+  return sign ? -n : n
+}
-- 
2.0.0.GIT

[PATCH 2/3] When generating CFI for i386 asm, don't mistake an FDIV instruction for DIV

[Alex Dowad]

...that would cause the script to erroneously conclude that the values of EAX
and EDX have been overwritten.

Probably won't make a difference, but it's just as well to get it right...
---
 tools/add-cfi.i386.awk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/add-cfi.i386.awk b/tools/add-cfi.i386.awk
index b8bdd7f..2da1899 100644
--- a/tools/add-cfi.i386.awk
+++ b/tools/add-cfi.i386.awk
@@ -190,7 +190,7 @@ function trashed(register) {
 }
 /i?mul [^,]*$/                    { trashed("eax"); trashed("edx") }
 /i?mul %e(ax|bx|cx|dx|si|di|bp),/ { trashed(get_reg1()) }
-/i?div/                           { trashed("eax"); trashed("edx") }
+/^i?div/                          { trashed("eax"); trashed("edx") }
 /(dec|inc|not|neg|pop) %e(ax|bx|cx|dx|si|di|bp)/  { trashed(get_reg()) }
 /cpuid/ { trashed("eax"); trashed("ebx"); trashed("ecx"); trashed("edx") }
 
-- 
2.0.0.GIT

[PATCH 3/3] Add script to add CFI directives to asm files in debug builds of x86_64

[Alex Dowad]

This makes debugging more pleasant for the odd times when you have to dig into
files implemented in x86_64 assembly.

It turns out that the parts we care about are almost exactly like i386, except
for the register names!
---
 tools/add-cfi.x86_64.awk | 185 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 185 insertions(+)
 create mode 100644 tools/add-cfi.x86_64.awk

diff --git a/tools/add-cfi.x86_64.awk b/tools/add-cfi.x86_64.awk
new file mode 100644
index 0000000..ed53fa5
--- /dev/null
+++ b/tools/add-cfi.x86_64.awk
@@ -0,0 +1,185 @@
+# Insert GAS CFI directives ("control frame information") into x86-64 asm input
+
+BEGIN {
+  # don't put CFI data in the .eh_frame ELF section (which we don't keep)
+  print ".cfi_sections .debug_frame"
+
+  # only emit CFI directives inside a function
+  in_function = 0
+
+  # emit .loc directives with line numbers from original source
+  printf ".file 1 \"%s\"\n", ARGV[1]
+  line_number = 0
+
+  # used to detect "call label; label:" trick
+  called = ""
+}
+
+function get_const1() {
+  # for instructions with 2 operands, get 1st operand (assuming it is constant)
+  match($0, /-?(0x[0-9a-fA-F]+|[0-9]+),/)
+  return parse_const(substr($0, RSTART, RLENGTH-1))
+}
+function get_reg() {
+  # only use if you already know there is 1 and only 1 register
+  match($0, /%r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15)/)
+  return substr($0, RSTART+1, RLENGTH-1)
+}
+function get_reg1() {
+  # for instructions with 2 operands, get 1st operand (assuming it is register)
+  match($0, /%r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15),/)
+  return substr($0, RSTART+1, RLENGTH-2)
+}
+function get_reg2() {
+  # for instructions with 2 operands, get 2nd operand (assuming it is register)
+  match($0, /,%r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15)/)
+  return substr($0, RSTART+2, RLENGTH-2)
+}
+
+function adjust_sp_offset(delta) {
+  if (in_function)
+    printf ".cfi_adjust_cfa_offset %d\n", delta
+}
+
+{
+  line_number = line_number + 1
+
+  # clean the input up before doing anything else
+  # delete comments
+  gsub(/(#|\/\/).*/, "")
+
+  # canonicalize whitespace
+  gsub(/[ \t]+/, " ") # mawk doesn't understand \s
+  gsub(/ *, */, ",")
+  gsub(/ *: */, ": ")
+  gsub(/ $/, "")
+  gsub(/^ /, "")
+}
+
+# check for assembler directives which we care about
+/^\.(section|data|text)/ {
+  # a .cfi_startproc/.cfi_endproc pair should be within the same section
+  # otherwise, clang will choke when generating ELF output
+  if (in_function) {
+    print ".cfi_endproc"
+    in_function = 0
+  }
+}
+/^\.type [a-zA-Z0-9_]+,\@function/ {
+  functions[substr($2, 1, length($2)-10)] = 1
+}
+# not interested in assembler directives beyond this, just pass them through
+/^\./ {
+  print
+  next
+}
+
+/^[a-zA-Z0-9_]+:/ {
+  label = substr($1, 1, length($1)-1) # drop trailing :
+
+  if (called == label) {
+    # note adjustment of stack pointer from "call label; label:"
+    adjust_sp_offset(8)
+  }
+
+  if (functions[label]) {
+    if (in_function)
+      print ".cfi_endproc"
+
+    in_function = 1
+    print ".cfi_startproc"
+
+    for (register in saved)
+      delete saved[register]
+    for (register in dirty)
+      delete dirty[register]
+  }
+
+  # an instruction may follow on the same line, so continue processing
+}
+
+/^$/ { next }
+
+{
+  called = ""
+  printf ".loc 1 %d\n", line_number
+  print
+}
+
+# KEEPING UP WITH THE STACK POINTER
+# %rsp should only be adjusted by pushing/popping or adding/subtracting constants
+#
+/pushl?/ {
+  adjust_sp_offset(8)
+}
+/popl?/ {
+  adjust_sp_offset(-8)
+}
+/addl? \$-?(0x[0-9a-fA-F]+|[0-9]+),%rsp/ { adjust_sp_offset(-get_const1()) }
+/subl? \$-?(0x[0-9a-fA-F]+|[0-9]+),%rsp/ { adjust_sp_offset(get_const1()) }
+
+/call/ {
+  if (match($0, /call [0-9]+f/)) # "forward" label
+    called = substr($0, RSTART+5, RLENGTH-6)
+  else if (match($0, /call [0-9a-zA-Z_]+/))
+    called = substr($0, RSTART+5, RLENGTH-5)
+}
+
+# TRACKING REGISTER VALUES FROM THE PREVIOUS STACK FRAME
+#
+/pushl? %r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15)/ { # don't match "push (%reg)"
+  # if a register is being pushed, and its value has not changed since the
+  #   beginning of this function, the pushed value can be used when printing
+  #   local variables at the next level up the stack
+  # emit '.cfi_rel_offset' for that
+
+  if (in_function) {
+    register = get_reg()
+    if (!saved[register] && !dirty[register]) {
+      printf ".cfi_rel_offset %s,0\n", register
+      saved[register] = 1
+    }
+  }
+}
+
+/movl? %r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15),-?(0x[0-9a-fA-F]+|[0-9]+)?\(%rsp\)/ {
+  if (in_function) {
+    register = get_reg()
+    if (match($0, /-?(0x[0-9a-fA-F]+|[0-9]+)\(%rsp\)/)) {
+      offset = parse_const(substr($0, RSTART, RLENGTH-6))
+    } else {
+      offset = 0
+    }
+    if (!saved[register] && !dirty[register]) {
+      printf ".cfi_rel_offset %s,%d\n", register, offset
+      saved[register] = 1
+    }
+  }
+}
+
+# IF REGISTER VALUES ARE UNCEREMONIOUSLY TRASHED
+# ...then we want to know about it.
+#
+function trashed(register) {
+  if (in_function && !saved[register] && !dirty[register]) {
+    printf ".cfi_undefined %s\n", register
+  }
+  dirty[register] = 1
+}
+# this does NOT exhaustively check for all possible instructions which could
+# overwrite a register value inherited from the caller (just the common ones)
+/mov.*,%r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15)/  { trashed(get_reg2()) }
+/(add|addl|sub|subl|and|or|xor|lea|sal|sar|shl|shr) %r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15),/ {
+  trashed(get_reg1())
+}
+/i?mul [^,]*$/                    { trashed("rax"); trashed("rdx") }
+/i?mul %r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15),/ { trashed(get_reg1()) }
+/^i?div/                          { trashed("rax"); trashed("rdx") }
+
+/(dec|inc|not|neg|pop) %r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15)/  { trashed(get_reg()) }
+/cpuid/ { trashed("rax"); trashed("rbx"); trashed("rcx"); trashed("rdx") }
+
+END {
+  if (in_function)
+    print ".cfi_endproc"
+}
-- 
2.0.0.GIT

Re: [PATCH 1/3] Pull a couple common AWK functions for CFI scripts into separate file

[Rich Felker]

On Fri, Oct 02, 2015 at 12:02:01PM +0200, Alex Dowad wrote:
> There is a lot which could be common between i386 and x86_64, but none of it
> will be useful for any other arch. These should be useful for all archs,
> however.
> ---
>  Makefile               |  2 +-
>  tools/add-cfi.i386.awk | 27 ---------------------------
>  tools/common.awk       | 26 ++++++++++++++++++++++++++
>  3 files changed, 27 insertions(+), 28 deletions(-)
>  create mode 100644 tools/common.awk
> 
> diff --git a/Makefile b/Makefile
> index 5a6a43b..761482d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -122,7 +122,7 @@ $(foreach s,$(wildcard src/*/$(ARCH)*/*.s),$(eval $(call mkasmdep,$(s))))
>  # Choose invocation of assembler to be used
>  # $(1) is input file, $(2) is output file, $(3) is assembler flags
>  ifeq ($(ADD_CFI),yes)
> -	AS_CMD = LC_ALL=C awk -f tools/add-cfi.$(ARCH).awk $< | $(CC) -x assembler -c -o $@ -
> +	AS_CMD = LC_ALL=C awk -f tools/common.awk -f tools/add-cfi.$(ARCH).awk $< | $(CC) -x assembler -c -o $@ -

Is the name common.awk appropriate? (common to what?) Maybe cfi-common
or something that indicates it's for the cfi scripts.

Great to see further work on this!

Rich

Re: [PATCH 3/3] Add script to add CFI directives to asm files in debug builds of x86_64

[Rich Felker]

On Fri, Oct 02, 2015 at 12:02:03PM +0200, Alex Dowad wrote:
> +# this does NOT exhaustively check for all possible instructions which could
> +# overwrite a register value inherited from the caller (just the common ones)
> +/mov.*,%r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15)/  { trashed(get_reg2()) }
> +/(add|addl|sub|subl|and|or|xor|lea|sal|sar|shl|shr) %r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15),/ {
> +  trashed(get_reg1())
> +}

This looks wrong but maybe it's already wrong in existing code. Why
are you marking the first operand as trashed rather than the second?

Rich

Re: [PATCH 3/3] Add script to add CFI directives to asm files in debug builds of x86_64

[Alex]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

On Fri, Oct 2, 2015 at 12:19 PM, Rich Felker <dalias@libc.org> wrote:

> On Fri, Oct 02, 2015 at 12:02:03PM +0200, Alex Dowad wrote:
> > +# this does NOT exhaustively check for all possible instructions which
> could
> > +# overwrite a register value inherited from the caller (just the common
> ones)
> > +/mov.*,%r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15)/  {
> trashed(get_reg2()) }
> > +/(add|addl|sub|subl|and|or|xor|lea|sal|sar|shl|shr)
> %r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15),/ {
> > +  trashed(get_reg1())
> > +}
>
> This looks wrong but maybe it's already wrong in existing code. Why
> are you marking the first operand as trashed rather than the second?


You're absolutely right!

[-- Attachment #2: Type: text/html, Size: 1083 bytes --]

Re: [PATCH 3/3] Add script to add CFI directives to asm files in debug builds of x86_64

[Alex]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

On Fri, Oct 2, 2015 at 12:58 PM, Alex <alexinbeijing@gmail.com> wrote:
>
> On Fri, Oct 2, 2015 at 12:19 PM, Rich Felker <dalias@libc.org> wrote:
>
>> On Fri, Oct 02, 2015 at 12:02:03PM +0200, Alex Dowad wrote:
>> > +# this does NOT exhaustively check for all possible instructions which
>> could
>> > +# overwrite a register value inherited from the caller (just the
>> common ones)
>> > +/mov.*,%r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15)/  {
>> trashed(get_reg2()) }
>> > +/(add|addl|sub|subl|and|or|xor|lea|sal|sar|shl|shr)
>> %r(ax|bx|cx|dx|si|di|bp|8|9|10|11|12|13|14|15),/ {
>> > +  trashed(get_reg1())
>> > +}
>>
>> This looks wrong but maybe it's already wrong in existing code. Why
>> are you marking the first operand as trashed rather than the second?
>
>
> You're absolutely right!
>

I'm pretty sure the reason I made this mistake was because of reading
Intel's developer manuals, where they write the destination register as the
first operand, and then absentmindedly applying the GNU syntax used by musl.

[-- Attachment #2: Type: text/html, Size: 1717 bytes --]