casaroli commented on code in PR #19938:
URL: https://github.com/apache/nuttx/pull/19938#discussion_r3841391302


##########
binfmt/elf.c:
##########
@@ -286,6 +290,28 @@ static int elf_loadbinary(FAR struct binary_s *binp,
     }
 #endif
 
+#ifdef CONFIG_BINFMT_CONSTRUCTORS
+  /* Run the constructors, as libelf_insert() does for dlopen().  A module
+   * with a PIC base is skipped: this task holds its own base, not the
+   * module's.
+   */
+
+  if (binp->picbase == NULL)

Review Comment:
   Yes, and that is `[8/10]` of this series. FDPIC gives a function pointer 
that carries its own data base, so the loader can enter a constructor with the 
module's base and put the caller's back after, through `fdpic_invoke()`. This 
guard is then replaced by that path.
   
   Plain PIC, without FDPIC, has no way to say which base one call needs, so a 
module with a D-Space is left as it is today, which is with no constructors at 
all. This PR does not make that case worse; it fixes the case that has no PIC 
base.



##########
binfmt/elf.c:
##########
@@ -286,6 +290,28 @@ static int elf_loadbinary(FAR struct binary_s *binp,
     }
 #endif
 
+#ifdef CONFIG_BINFMT_CONSTRUCTORS
+  /* Run the constructors, as libelf_insert() does for dlopen().  A module
+   * with a PIC base is skipped: this task holds its own base, not the
+   * module's.
+   */
+
+  if (binp->picbase == NULL)
+    {
+      array = (FAR void (**)(void))loadinfo.preiarr;

Review Comment:
   You are right, and I have pushed a fix.
   
   An executable is now skipped, because `crt0.c` calls the same array: 
`libs/libc/elf/gnu-elf.ld` puts `.init_array` and `.ctors` between `_sctors` 
and `_ectors`, and `exec_ctors()` walks exactly that, on the task that runs the 
module and in its address environment.
   
   A build with `CONFIG_ARCH_ADDRENV` is skipped for the same reason. The 
address environment of the module is not selected at that point in 
`elf_loadbinary()`, so this task cannot reach the array at all, and such a 
build loads an executable, thus `crt0` covers it.
   
   What is left is the relocatable module in a flat build. It has no `crt0`, 
nothing calls its `.init_array`, and there is no address environment to enter. 
`libelf_insert()` already calls the array from the loading task for the same 
object arriving through `dlopen()`, so this only makes `exec()` agree with 
`dlopen()`.
   
   The guard is now:
   
   ```c
   #if defined(CONFIG_BINFMT_CONSTRUCTORS) && !defined(CONFIG_ARCH_ADDRENV)
     if (loadinfo.ehdr.e_type != ET_EXEC && binp->picbase == NULL)
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to