Re: [PATCH] SEH: Reimplement the .seh_scope directive

Re: [PATCH] SEH: Reimplement the .seh_scope directive

[Julian Waters]

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

Anyone?

Re: [PATCH] SEH: Reimplement the .seh_scope directive

[Nick Clifton]

Hi Julian,

> 3. Where should the documentation be?

gas/doc/c-i386.texi.  In the i386-Options section.
(I am right in thinking that these directives are only used for x86_64 PE format binaries, right ?)

> I know what some (but not all) of the directives do, so I'll be happy to document the ones I know

Please do, that would be very helpful.

I also noticed that the directives are mentioned in a comment at the start
of gas/config/obj-coff-seh.h, so if you could add a line for .seh_scope there too
that would be nice.

Cheers  Nick

Re: [PATCH] SEH: Reimplement the .seh_scope directive

[Julian Waters]

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

Hi Nick, thanks for chipping in,

Sorry for dragging you into this, it's just that I thought you were the
best person to look to since you authored the .seh_code directive some time
(9 years!!) ago :P

0. I don't mind submitting this under whatever license, so long as I can
contribute it, I'm happy

1. Ah, I thought I could get away with that warning, I didn't know binutils
was compiled with -Werror on. I'll fix it, thanks for the heads up

2. I'm not too sure how to check if there are more than 4, I'll get back to
you on that

3. Where should the documentation be? I know what some (but not all) of the
directives do, so I'll be happy to document the ones I know

4. Ah, the joys of auto formatting. I'll try to get rid of all of them in
the final patch

Thanks for getting back to me, I appreciate the time! :)

best regards,
Julian

On Wed, Jul 19, 2023 at 6:53 PM Nick Clifton <nickc@redhat.com> wrote:

> Hi Julian,
>
> > Anyone? Some reviews would help, maybe Nick or Alan?
>
> Sorry for the delay in reviewing your patch.
>
> Unfortunately there are a few problems:
>
> 0.  Do you have an FSF Copyright assignment for contributions
>      to the GNU Binutils project.  Or are you willing to submit
>      the patch under the terms of a Developer's Certificate of
>      Origin ?
>
> 1. The patch does not compile:
>
> In file included from gas/config/obj-coff.c:57:
> gas/config/obj-coff-seh.c: In function 'obj_coff_seh_scope':
> gas/config/obj-coff-seh.c:406:29: error: comparison between pointer and
> integer [-Werror]
>    406 |     if (*input_line_pointer == NULL || *input_line_pointer ==
> '\n') {
>        |                             ^^
>
> 2. The code does not check to see if more than 4 arguments have been
>     supplied to the .seh_scope directive.
>
> 3. There ought to be some description of the .seh_scope directive in
>     the assembler's documentation.   (Although it appears that none
>     of the directives are documented, which is naughty).  If your patch
>     were to add some then this might encourage others to extend the
>     coverage to the other directives.
>
> 4. There are various (minor) formatting issues that should be fixed
>     such as the indentation, unnecessary blank lines, long lines and
>     so on.  Please have a look at the GNU Coding Standards for more
>     information:  https://www.gnu.org/prep/standards/
>
> Cheers
>    Nick
>
>

Re: [PATCH] SEH: Reimplement the .seh_scope directive

[Nick Clifton]

Hi Julian,

> Anyone? Some reviews would help, maybe Nick or Alan?

Sorry for the delay in reviewing your patch.

Unfortunately there are a few problems:

0.  Do you have an FSF Copyright assignment for contributions
     to the GNU Binutils project.  Or are you willing to submit
     the patch under the terms of a Developer's Certificate of
     Origin ?

1. The patch does not compile:

In file included from gas/config/obj-coff.c:57:
gas/config/obj-coff-seh.c: In function 'obj_coff_seh_scope':
gas/config/obj-coff-seh.c:406:29: error: comparison between pointer and integer [-Werror]
   406 |     if (*input_line_pointer == NULL || *input_line_pointer == '\n') {
       |                             ^^

2. The code does not check to see if more than 4 arguments have been
    supplied to the .seh_scope directive.

3. There ought to be some description of the .seh_scope directive in
    the assembler's documentation.   (Although it appears that none
    of the directives are documented, which is naughty).  If your patch
    were to add some then this might encourage others to extend the
    coverage to the other directives.

4. There are various (minor) formatting issues that should be fixed
    such as the indentation, unnecessary blank lines, long lines and
    so on.  Please have a look at the GNU Coding Standards for more
    information:  https://www.gnu.org/prep/standards/

Cheers
   Nick

Re: [PATCH] SEH: Reimplement the .seh_scope directive

[Julian Waters]

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

Anyone? Some reviews would help, maybe Nick or Alan?

best regards,
Julian

[PATCH] SEH: Reimplement the .seh_scope directive

[Julian Waters]

[-- Attachment #1.1: Type: text/plain, Size: 361 bytes --]

The .seh_scope directive was removed a little over a decade ago due to
being too Microsoft specific. This has proven to be a mistake as there is
now no easy way for reusable inline assembly to express Structured
Exception Handling scopes for Microsoft Windows targets. This patch
reimplements a simpler version of .seh_scope, with the proper semantics in
place

[-- Attachment #2: 0001-Reimplement-the-.seh_scope-directive.patch --]
[-- Type: application/octet-stream, Size: 4644 bytes --]

From 18da45e966a7f1cc91782517e3c992cbf52cd974 Mon Sep 17 00:00:00 2001
From: TheShermanTanker <tanksherman27@gmail.com>
Date: Fri, 7 Jul 2023 16:23:07 +0800
Subject: [PATCH] Reimplement the .seh_scope directive

---
 gas/config/obj-coff-seh.c | 100 +++++++++++++++++++++++++++++++++++---
 gas/config/obj-coff-seh.h |  16 +++++-
 2 files changed, 108 insertions(+), 8 deletions(-)

diff --git a/gas/config/obj-coff-seh.c b/gas/config/obj-coff-seh.c
index 7b4486a348f..661c3d61137 100644
--- a/gas/config/obj-coff-seh.c
+++ b/gas/config/obj-coff-seh.c
@@ -388,6 +388,69 @@ obj_coff_seh_handlerdata (int what ATTRIBUTE_UNUSED)
   switch_xdata (seh_ctx_cur->subsection + 1, seh_ctx_cur->code_seg);
 }
 
+static void
+obj_coff_seh_scope (int)
+{
+  if (!verify_context_and_target (".seh_scope", seh_kind_x64))
+    return;
+
+  seh_ctx_cur->scopes += 1;
+
+  expressionS addrs[4];
+
+  for (unsigned short i = 0; i < 4; i += 1) {
+    while (*input_line_pointer == ' ' || *input_line_pointer == '\t' || *input_line_pointer == ',') {
+      input_line_pointer += 1;
+    }
+
+    if (*input_line_pointer == NULL || *input_line_pointer == '\n') {
+        as_fatal (_(".seh_scope requires four symbol names.\n"));
+        demand_empty_rest_of_line ();
+        return;
+    }
+
+    expressionS exp;
+
+    if (*input_line_pointer == '@' && i > 1) {
+      char* name;
+      char end;
+      end = get_symbol_name (&name);
+      if (strcasecmp (name, "@null") == 0) {
+        exp.X_op = O_constant;
+        exp.X_add_number = 0;
+      } else {
+        as_fatal (_(".seh_scope encountered an unknown constant\n"));
+        demand_empty_rest_of_line ();
+        return;
+      }
+      restore_line_pointer (end);
+    } else {
+      expression(&exp);
+      exp.X_op = O_symbol_rva;
+    }
+
+    addrs[i] = exp;
+  }
+
+  seh_scope scope;
+
+  scope.start = addrs[0];
+  scope.end = addrs[1];
+  scope.filter = addrs[2];
+  scope.handler = addrs[3];
+
+  if (seh_ctx_cur->scopeinfo == NULL) {
+    seh_ctx_cur->scopeinfo = XCNEW (seh_scope);
+  } else {
+    seh_scope* newarray = XCNEWVEC (seh_scope, seh_ctx_cur->scopes);
+    memcpy (newarray, seh_ctx_cur->scopeinfo, (seh_ctx_cur->scopes - 1) * sizeof (seh_scope));
+    XDELETEVEC (seh_ctx_cur->scopeinfo);
+    seh_ctx_cur->scopeinfo = newarray;
+  }
+
+  memcpy (seh_ctx_cur->scopeinfo + (seh_ctx_cur->scopes - 1), &scope, sizeof (seh_scope));
+}
+
 /* Mark end of current context.  */
 
 static void
@@ -902,6 +965,25 @@ seh_x64_write_function_xdata (seh_context *c)
   /* Handler data will be tacked in here by subsections.  */
 }
 
+static void
+seh_x64_maybe_emit_scope_info (seh_context *c)
+{
+
+  if (c->scopeinfo != NULL) {
+    out_four (c->scopes);
+    for (unsigned short i = 0; i < c->scopes; i += 1) {
+      seh_scope scope = *(c->scopeinfo + i);
+      emit_expr (&(scope.start), 4);
+      emit_expr (&(scope.end), 4);
+      emit_expr (&(scope.filter), 4);
+      emit_expr (&(scope.handler), 4);
+    }
+
+    XDELETEVEC (c->scopeinfo);
+  }
+
+}
+
 /* Write out xdata for one function.  */
 
 static void
@@ -918,6 +1000,10 @@ write_function_xdata (seh_context *c)
 
   seh_x64_write_function_xdata (c);
 
+  switch_xdata (c->subsection + 1, c->code_seg);
+
+  seh_x64_maybe_emit_scope_info (c);
+
   subseg_set (save_seg, save_subseg);
 }
 
diff --git a/gas/config/obj-coff-seh.h b/gas/config/obj-coff-seh.h
index 8d77bacff71..f83e99c3c65 100644
--- a/gas/config/obj-coff-seh.h
+++ b/gas/config/obj-coff-seh.h
@@ -75,7 +75,8 @@
 	{"seh_no32", obj_coff_seh_32, 0}, \
 	{"seh_handler", obj_coff_seh_handler, 0}, \
 	{"seh_code", obj_coff_seh_code, 0}, \
-	{"seh_handlerdata", obj_coff_seh_handlerdata, 0},
+	{"seh_handlerdata", obj_coff_seh_handlerdata, 0}, \
+	{"seh_scope", obj_coff_seh_scope, 0},
 
 /* Type definitions.  */
 
@@ -87,6 +88,14 @@ typedef struct seh_prologue_element
   symbolS *pc_addr;
 } seh_prologue_element;
 
+typedef struct seh_scope
+{
+  expressionS start;
+  expressionS end;
+  expressionS filter;
+  expressionS handler;
+} seh_scope;
+
 typedef struct seh_context
 {
   struct seh_context *next;
@@ -128,6 +137,10 @@ typedef struct seh_context
   int elems_count;
   int elems_max;
   seh_prologue_element *elems;
+
+  /* Information for .seh_scope on x64 */
+  unsigned int scopes;
+  seh_scope *scopeinfo;
 } seh_context;
 
 typedef enum seh_kind {
@@ -151,6 +164,7 @@ static void obj_coff_seh_proc  (int);
 static void obj_coff_seh_handler (int);
 static void obj_coff_seh_handlerdata (int);
 static void obj_coff_seh_code (int);
+static void obj_coff_seh_scope (int);
 
 #define UNDSEC bfd_und_section_ptr
 
-- 
2.35.1.windows.2