You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/21 04:57:22 UTC

[GitHub] [incubator-nuttx] masayuki2009 opened a new pull request, #6651: binfmt: Fix a memory leank in ELF loader

masayuki2009 opened a new pull request, #6651:
URL: https://github.com/apache/incubator-nuttx/pull/6651

   ## Summary
   
   - With spresense:elf, it uses CONFIG_ARCH_USE_TEXT_HEAP=y.
   - In this configuration, it would be complicated if we use existing
     alloc[] in 'struct binary_s' to release the data section.
   - That's why I added a new member for it.
   
   ## Impact
   
   - ELF loader with CONFIG_ARCH_ADDRENV=n
   
   ## Testing
   
   - Tested with the following configs
     - spresense:elf, spresense:wifi_smp
     - rv-virt:nsh64, sabre-6quad:netnsh
   


-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6651: binfmt: Fix a memory leank in ELF loader

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6651:
URL: https://github.com/apache/incubator-nuttx/pull/6651#issuecomment-1192189843

   I'll update this PR later based on @pkarashchenko 's suggestion.
   


-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6651: binfmt: Fix a memory leank in ELF loader

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6651:
URL: https://github.com/apache/incubator-nuttx/pull/6651#issuecomment-1191448725

   @masayuki2009 I think that is not only related to data segment as `ctoralloc` and `dtoralloc` also seems to use `kumm_malloc()` and not `up_textheap_memalign()`, so maybe `textalloc` should use special handling and not `dataalloc`? I mean maybe we can get your previous fix, but modify memory release code to be
   ```
         for (i = 0; i < BINFMT_NALLOC; i++)
           {
             if (binp->alloc[i])
               {
                 binfo("Freeing alloc[%d]: %p\n", i, binp->alloc[i]);
   #if defined(CONFIG_ARCH_USE_TEXT_HEAP)
                 if (i == 0)
                   {
                     up_textheap_free((FAR void *)binp->alloc[i]);
                   }
                 else
   #endif
                   {
                     kumm_free((FAR void *)binp->alloc[i]);
                   }
               }
           }
   ```


-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6651: binfmt: Fix a memory leank in ELF loader

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6651:
URL: https://github.com/apache/incubator-nuttx/pull/6651#issuecomment-1192115411

   ctoralloc/dtoralloc section contain the function pointer which is more like the data not code, but it's always better to confirm with @yamt who introduce the text heap to fix the Harvard architecture issue found on xtensa.


-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6651: binfmt: Fix a memory leank in ELF loader

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6651:
URL: https://github.com/apache/incubator-nuttx/pull/6651#issuecomment-1192189155

   >ctoralloc/dtoralloc section contain the function pointer which is more like the data than the code, but it's always better to confirm with @yamt who introduce the text heap to fix the Harvard architecture issue found on xtensa.
   
   @xiaoxiang781216 
   I confirmed that you are correct. To test C++ static object in ELF, I created a new PR https://github.com/apache/incubator-nuttx/pull/6684


-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6651: binfmt: Fix a memory leak in ELF loader

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6651:
URL: https://github.com/apache/incubator-nuttx/pull/6651


-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6651: binfmt: Fix a memory leank in ELF loader

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6651:
URL: https://github.com/apache/incubator-nuttx/pull/6651#issuecomment-1191038848

   See https://github.com/apache/incubator-nuttx/pull/6647 as well
   


-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6651: binfmt: Fix a memory leank in ELF loader

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6651:
URL: https://github.com/apache/incubator-nuttx/pull/6651#issuecomment-1193050627

   >I'll update this PR later based on @pkarashchenko 's suggestion.
   
   @xiaoxiang781216 
   I've just pushed the commit and also updated the Summary and the Testing


-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6651: binfmt: Fix a memory leank in ELF loader

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6651:
URL: https://github.com/apache/incubator-nuttx/pull/6651#issuecomment-1192054429

   >@masayuki2009 I think that is not only related to data segment as ctoralloc and dtoralloc also seems to use kumm_malloc() and not up_textheap_memalign(),
   
   @pkarashchenko 
   I think ctoralloc and dtoralloc should be allocated by up_textheap_memalign() if CONFIG_ARCH_USE_TEXT_HEAP=y.
   However, let me confirm them later.


-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org