[lld][COFF] Remove incorrect flag from EHcont table

[namazso]

Summary

See https://github.com/llvm/llvm-project/issues/64570

[pengfei]

Can you add a multi entries test to show the change, please?

[namazso]

Updated objdump, added a test.

[rnk]

Thanks!

llvm/tools/llvm-readobj/COFFDumper.cpp

864

You can simplify the code and eliminate all these else clauses by changing this condition above in printRVATable:

  if (PrintExtra)
      PrintExtra(OS, reinterpret_cast<const uint8_t *>(I));
>>
  if (EntrySize == 5)
      PrintExtra(OS, reinterpret_cast<const uint8_t *>(I));

It's not general to more than a byte of flags, but that's true already.

[namazso]

Replaced the previous ehcont test with the new one, as it tests nothing the new one doesn't

@rnk Removed the duplicated ifs. Technically the flags are not defined at all for anything but the CF table, so we could straight up just ignore them, but we don't decode them in the CF table either and instead just dump them out in hex, so might as well just do the same for the rest.

[rnk]

lgtm

[mstorsjo]

llvm/tools/llvm-readobj/COFFDumper.cpp

947

This broke compilation with GCC:

../tools/llvm-readobj/COFFDumper.cpp: In member function ‘virtual void {anonymous}::COFFDumper::printCOFFLoadConfig()’:
../tools/llvm-readobj/COFFDumper.cpp:947:41: error: operands to ‘?:’ have different types ‘{anonymous}::COFFDumper::printCOFFLoadConfig()::<lambda(llvm::raw_ostream&, const uint8_t*)>’ and ‘std::nullptr_t’
  947 |   PrintExtraCB PrintExtra = Stride == 1 ? PrintGuardFlags : nullptr;
      |                             ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~

[duck-37]

llvm/tools/llvm-readobj/COFFDumper.cpp

947

Quick-fix pushed up just now. Seems to have worked.

[rnk]

You can see where the eh cont table was added here: https://reviews.llvm.org/D99078

lld/COFF/Writer.cpp

1799

I think with this change the final parameter of maybeAddRVATable is completely dead. Would you please make a follow up change to remove the code for it?

1876

We don't need RVAFlagTableChunk anymore.

[namazso]

lld/COFF/Writer.cpp

1799

I wasn't sure about throwing out all that code since strides exist for a reason, which is the suppression flags (currently unsupported by lld). I guess if (when) someone starts caring about them they can just add it back, in some actually usable way, as in the current one you can't actually control the contents (it's always 0).

Will make a follow-up with the removals.

lld/COFF/Writer.cpp

   // Add the longjmp target table unless the user told us not to.
   if (config->guardCF & GuardCFLevel::LongJmp)
     maybeAddRVATable(std::move(longJmpTargets), "__guard_longjmp_table",
                      "__guard_longjmp_count");
 
   // Add the ehcont target table unless the user told us not to.
   if (config->guardCF & GuardCFLevel::EHCont)
     maybeAddRVATable(std::move(ehContTargets), "__guard_eh_cont_table",
-                     "__guard_eh_cont_count", true);
+                     "__guard_eh_cont_count");
 
   // Set __guard_flags, which will be used in the load config to indicate that
   // /guard:cf was enabled.
   uint32_t guardFlags = uint32_t(GuardFlags::CF_INSTRUMENTED) |
                         uint32_t(GuardFlags::CF_FUNCTION_TABLE_PRESENT);
   if (config->guardCF & GuardCFLevel::LongJmp)
     guardFlags |= uint32_t(GuardFlags::CF_LONGJUMP_TABLE_PRESENT);
   if (config->guardCF & GuardCFLevel::EHCont)
     guardFlags |= uint32_t(GuardFlags::EH_CONTINUATION_TABLE_PRESENT);
   Symbol *flagSym = ctx.symtab.findUnderscore("__guard_flags");
   cast<DefinedAbsolute>(flagSym)->setVA(guardFlags);
 }
 
 // Take a list of input sections containing symbol table indices and add those
 // symbols to a vector. The challenge is that symbol RVAs are not known and
 // depend on the table size, so we can't directly build a set of integers.

lld/test/COFF/guard-ehcont.s

 # CHECK:   GuardFlags [ (0x400500)
 # CHECK:     CF_FUNCTION_TABLE_PRESENT (0x400)
 # CHECK:     CF_INSTRUMENTED (0x100)
 # CHECK:     EH_CONTINUATION_TABLE_PRESENT (0x400000)
 # CHECK:   ]
 # CHECK:   GuardAddressTakenIatEntryTable: 0x0
 # CHECK:   GuardAddressTakenIatEntryCount: 0
 # CHECK:   GuardEHContinuationTable: 0x14000{{.*}}
-# CHECK:   GuardEHContinuationCount: 1
+# CHECK:   GuardEHContinuationCount: 2
 # CHECK: ]
 # CHECK:      GuardEHContTable [
 # CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT:   0x14000{{.*}}
 # CHECK-NEXT: ]
 
-
-# This assembly is reduced from C code like:
-# int main()
-# {
-#   try {
-#     throw 3;
-#   }
-#   catch (int e) {
-#     return e != 3;
-#   }
-#   return 2;
-# }
-
 # We need @feat.00 to have 0x4000 to indicate /guard:ehcont.
         .def     @feat.00;
         .scl    3;
         .type   0;
         .endef
         .globl  @feat.00
 @feat.00 = 0x4000
         .def     main; .scl    2; .type   32; .endef
         .globl  main                            # -- Begin function main
         .p2align  4, 0x90
-main:                                   # @main
-.Lfunc_begin0:
+main:
 .seh_proc main
-.intel_syntax
-        .seh_handler __CxxFrameHandler3, @unwind, @except
-# %bb.0:                                # %entry
-        push  rbp
-        .seh_pushreg rbp
-        sub rsp, 64
-        .seh_stackalloc 64
-        lea rbp, [rsp + 64]
-        .seh_setframe rbp, 64
-        .seh_endprologue
-        mov qword ptr [rbp - 16], -2
-        mov dword ptr [rbp - 20], 0
-        mov dword ptr [rbp - 24], 3
-.Ltmp0:
-        lea rdx, [rip + _TI1H]
-        lea rcx, [rbp - 24]
-        call  _CxxThrowException
-.Ltmp1:
-        jmp .LBB0_3
-.LBB0_2:                                # Block address taken
-                                        # %catchret.dest
-$ehgcr_0_2:
-        mov  eax, dword ptr [rbp - 20]
-        add rsp, 64
-        pop rbp
-        ret
-.LBB0_3:                                # %unreachable
-        int3
+        .seh_handler __C_specific_handler, @unwind, @except
         .seh_handlerdata
-        .long  ($cppxdata$main)@IMGREL
+        .long 2
+        .long (seh_begin)@IMGREL
+        .long (seh_end)@IMGREL
+        .long 1
+        .long (seh_except)@IMGREL
+        .long (seh2_begin)@IMGREL
+        .long (seh2_end)@IMGREL
+        .long 1
+        .long (seh2_except)@IMGREL
         .text
-        .seh_endproc
-        .def   "?catch$1@?0?main@4HA";
-        .scl  3;
-        .type  32;
-        .endef
-        .p2align  4, 0x90
-"?catch$1@?0?main@4HA":
-.seh_proc "?catch$1@?0?main@4HA"
-        .seh_handler __CxxFrameHandler3, @unwind, @except
-.LBB0_1:                                # %catch
-        mov  qword ptr [rsp + 16], rdx
-        push  rbp
-        .seh_pushreg rbp
-        sub  rsp, 32
-        .seh_stackalloc 32
-        lea  rbp, [rdx + 64]
-        .seh_endprologue
-        mov  eax, dword ptr [rbp - 4]
-        sub eax, 3
-        setne al
-        movzx eax, al
-        mov dword ptr [rbp - 20], eax
-        lea rax, [rip + .LBB0_2]
-        add rsp, 32
-        pop rbp
-        ret                                     # CATCHRET
-        .def     free; .scl    2; .type   32; .endef
-        .globl  free
-free:
-        ret
-        .def     __CxxFrameHandler3; .scl    2; .type   32; .endef
-        .globl  __CxxFrameHandler3
-__CxxFrameHandler3:
-        ret
-        .def     _CxxThrowException; .scl    2; .type   32; .endef
-        .globl  _CxxThrowException
-_CxxThrowException:
+    seh_begin:
+        nop
+        int3
+        nop
+    seh_end:
+        nop
+    seh_except:
+        nop
+
+    seh2_begin:
+        nop
+        int3
+        nop
+    seh2_end:
+        nop
+    seh2_except:
+        nop
+
+        xor %eax, %eax
         ret
-.Lfunc_end0:
-        .seh_handlerdata
-        .long ($cppxdata$main)@IMGREL
-        .text
-        .seh_endproc
-        .section  .xdata,"dr"
-        .p2align  2
-$cppxdata$main:
-        .long 429065506                       # MagicNumber
-        .long 2                               # MaxState
-        .long ($stateUnwindMap$main)@IMGREL   # UnwindMap
-        .long 1                               # NumTryBlocks
-        .long ($tryMap$main)@IMGREL           # TryBlockMap
-        .long 4                               # IPMapEntries
-        .long ($ip2state$main)@IMGREL         # IPToStateXData
-        .long 48                              # UnwindHelp
-        .long 0                               # ESTypeList
-        .long 1                               # EHFlags
-$stateUnwindMap$main:
-        .long -1                              # ToState
-        .long 0                               # Action
-        .long -1                              # ToState
-        .long 0                               # Action
-$tryMap$main:
-        .long 0                               # TryLow
-        .long 0                               # TryHigh
-        .long 1                               # CatchHigh
-        .long 1                               # NumCatches
-        .long ($handlerMap$0$main)@IMGREL     # HandlerArray
-$handlerMap$0$main:
-        .long 0                               # Adjectives
-        .long "??_R0H@8"@IMGREL               # Type
-        .long 60                              # CatchObjOffset
-        .long "?catch$1@?0?main@4HA"@IMGREL   # Handler
-        .long 56                              # ParentFrameOffset
-$ip2state$main:
-        .long .Lfunc_begin0@IMGREL            # IP
-        .long -1                              # ToState
-        .long .Ltmp0@IMGREL+1                 # IP
-        .long 0                               # ToState
-        .long .Ltmp1@IMGREL+1                 # IP
-        .long -1                              # ToState
-        .long "?catch$1@?0?main@4HA"@IMGREL   # IP
-        .long 1                               # ToState
-        .text
-                                        # -- End function
-        .section  .data,"dw",discard,"??_R0H@8"
-        .globl  "??_R0H@8"                      # @"??_R0H@8"
-        .p2align  4
-"??_R0H@8":
-        .quad 0
-        .quad 0
-        .asciz  ".H"
-        .zero 5
-
-        .section  .xdata,"dr",discard,"_CT??_R0H@84"
-        .globl  "_CT??_R0H@84"                  # @"_CT??_R0H@84"
-        .p2align  4
-"_CT??_R0H@84":
-        .long 1                               # 0x1
-        .long "??_R0H@8"@IMGREL
-        .long 0                               # 0x0
-        .long 4294967295                      # 0xffffffff
-        .long 0                               # 0x0
-        .long 4                               # 0x4
-        .long 0                               # 0x0
-
-        .section  .xdata,"dr",discard,_CTA1H
-        .globl  _CTA1H                          # @_CTA1H
-        .p2align  3
-_CTA1H:
-        .long 1                               # 0x1
-        .long "_CT??_R0H@84"@IMGREL
-
-        .section  .xdata,"dr",discard,_TI1H
-        .globl  _TI1H                           # @_TI1H
-        .p2align  3
-_TI1H:
-        .long 0                               # 0x0
-        .long 0                               # 0x0
-        .long 0                               # 0x0
-        .long _CTA1H@IMGREL
+
+__C_specific_handler:
+        ret
 
-        .section  .gehcont$y,"dr"
-        .symidx $ehgcr_0_2
-        .addrsig_sym _CxxThrowException
-        .addrsig_sym __CxxFrameHandler3
-        .addrsig_sym "??_R0H@8"
-        .addrsig_sym __ImageBase
-        .section  .rdata,"dr"
+.symidx seh_except
+.symidx seh2_except
+
 .globl _load_config_used
 _load_config_used:
         .long 312
         .fill 124, 1, 0
         .quad __guard_fids_table
         .quad __guard_fids_count
         .long __guard_flags
         .fill 12, 1, 0
         .quad __guard_iat_table
         .quad __guard_iat_count
         .quad __guard_longjmp_table
         .quad __guard_longjmp_count
         .fill 72, 1, 0
         .quad __guard_eh_cont_table
         .quad __guard_eh_cont_count
-        .fill 32, 1, 0
+        .fill 32, 1, 0
 

llvm/tools/llvm-readobj/COFFDumper.cpp

   }
 
   auto PrintGuardFlags = [](raw_ostream &OS, const uint8_t *Entry) {
     uint8_t Flags = *reinterpret_cast<const uint8_t *>(Entry + 4);
     if (Flags)
       OS << " flags " << utohexstr(Flags);
   };
 
+  // The stride gives the number of extra bytes in addition to the 4-byte
+  // RVA of each entry in the table. As of writing only a 1-byte extra flag
+  // has been defined.
+  uint32_t Stride = Tables.GuardFlags >> 28;
+  PrintExtraCB PrintExtra = Stride == 1 ? PrintGuardFlags : nullptr;
+
   if (Tables.GuardFidTableVA) {
     ListScope LS(W, "GuardFidTable");
-    if (uint32_t Size =
-            Tables.GuardFlags &
-            uint32_t(COFF::GuardFlags::CF_FUNCTION_TABLE_SIZE_MASK)) {
-      // The size mask gives the number of extra bytes in addition to the 4-byte
-      // RVA of each entry in the table. As of writing only a 1-byte extra flag
-      // has been defined.
-      Size = (Size >> 28) + 4;
-      printRVATable(Tables.GuardFidTableVA, Tables.GuardFidTableCount, Size,
-                    PrintGuardFlags);
-    } else {
-      printRVATable(Tables.GuardFidTableVA, Tables.GuardFidTableCount, 4);
-    }
+    printRVATable(Tables.GuardFidTableVA, Tables.GuardFidTableCount,
+                  4 + Stride, PrintExtra);
   }
 
   if (Tables.GuardIatTableVA) {
     ListScope LS(W, "GuardIatTable");
-    printRVATable(Tables.GuardIatTableVA, Tables.GuardIatTableCount, 4);
+    printRVATable(Tables.GuardIatTableVA, Tables.GuardIatTableCount,
+                  4 + Stride, PrintExtra);
   }
 
   if (Tables.GuardLJmpTableVA) {
     ListScope LS(W, "GuardLJmpTable");
-    printRVATable(Tables.GuardLJmpTableVA, Tables.GuardLJmpTableCount, 4);
+    printRVATable(Tables.GuardLJmpTableVA, Tables.GuardLJmpTableCount,
+                  4 + Stride, PrintExtra);
   }
 
   if (Tables.GuardEHContTableVA) {
     ListScope LS(W, "GuardEHContTable");
-    printRVATable(Tables.GuardEHContTableVA, Tables.GuardEHContTableCount, 5,
-                  PrintGuardFlags);
+    printRVATable(Tables.GuardEHContTableVA, Tables.GuardEHContTableCount,
+                  4 + Stride, PrintExtra);
   }
 }