Skip to content

Conversation

@keith-packard
Copy link

Only apply alignment restrictions to VMA and not LMA when re-writing the elf header by removing the alignment adjustment when validating the first section address.

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be trying to fix the same problem that 81f63d1 fixes in a slightly different way.

Only apply alignment restrictions to VMA and not LMA when re-writing
the elf header by removing the alignment adjustment when validating
the first section address.

I do not recall all the details; but, is this universally correct?

I assume whoever wrote this put align_power in case there are file headers or program headers in a segment whose size may not exactly fall at the section alignment ... but then, we are still comparing LMAs and not VMAs here so it would not be correct either way.

@keith-packard
Copy link
Author

This seems to be trying to fix the same problem that 81f63d1 fixes in a slightly different way.

Nice to see other people landing on the same issue and looking into this. I don't know whether alignment is ever supposed to be applied to LMA, but maybe it is? I can imagine needing to round LMA up to some access control boundary, but I don't see how you could separately specify LMA and VMA alignment values. But, your patch has a useful change -- it doesn't fail when the LMA is following the alignment value, and that seems like it will cause fewer false failures. Do you think we need to allow for file headers and phdrs to be included in this computation?

Only apply alignment restrictions to VMA and not LMA when re-writing
the elf header by removing the alignment adjustment when validating
the first section address.

I do not recall all the details; but, is this universally correct?

When LMA and VMA are the same, then it's probably not correct, so we should allow either? Does anyone know a serious ELF expert who could answer this question?

I assume whoever wrote this put align_power in case there are file headers or program headers in a segment whose size may not exactly fall at the section alignment ... but then, we are still comparing LMAs and not VMAs here so it would not be correct either way.

Yeah, it seemed mysterious to me, but I'm looking at it from the perspective of one particular issue. Come back in a year with a different bug and I suspect I'd be mystified all over again.

Allow the first section LMA to match either the raw LMA of the
segment or the alignment-adjusted LMA of the segment. I'm not sure why
the LMA should ever be adjusted with the VMA alignment, but this change
doesn't affect the output when things work, it only makes objcopy
less strict.

Signed-off-by: Keith Packard <keithp@keithp.com>
Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least, we are now allowing the aligned address preserving the existing behaviour (of course, its correctness is debatable), so this should be less problematic overall.

@stephanosio stephanosio merged commit 5f3358c into zephyrproject-rtos:zephyr-binutils-2_43_1 Jul 7, 2025
@keith-packard keith-packard deleted the objcopy-no-align-lma branch July 8, 2025 16:49
duynguyenxa pushed a commit to renesas/binutils-gdb that referenced this pull request Oct 29, 2025
On Windows gcore is not implemented, and if you try it, you get an
heap-use-after-free error:

(gdb) gcore C:/gdb/build64/gdb-git-python3/gdb/testsuite/outputs/gdb.base/gcore-buffer-overflow/gcore-buffer-overflow.test
warning: cannot close "=================================================================
==10108==ERROR: AddressSanitizer: heap-use-after-free on address 0x1259ea503110 at pc 0x7ff6806e3936 bp 0x0062e01ed990 sp 0x0062e01ed140
READ of size 111 at 0x1259ea503110 thread T0
    #0 0x7ff6806e3935 in strlen C:/gcc/src/gcc-14.2.0/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:391
    zephyrproject-rtos#1 0x7ff6807169c4 in __pformat_puts C:/gcc/src/mingw-w64-v12.0.0/mingw-w64-crt/stdio/mingw_pformat.c:558
    zephyrproject-rtos#2 0x7ff6807186c1 in __mingw_pformat C:/gcc/src/mingw-w64-v12.0.0/mingw-w64-crt/stdio/mingw_pformat.c:2514
    zephyrproject-rtos#3 0x7ff680713614 in __mingw_vsnprintf C:/gcc/src/mingw-w64-v12.0.0/mingw-w64-crt/stdio/mingw_vsnprintf.c:41
    zephyrproject-rtos#4 0x7ff67f34419f in vsnprintf(char*, unsigned long long, char const*, char*) C:/msys64/mingw64/x86_64-w64-mingw32/include/stdio.h:484
    zephyrproject-rtos#5 0x7ff67f34419f in string_vprintf[abi:cxx11](char const*, char*) C:/gdb/src/gdb.git/gdbsupport/common-utils.cc:106
    zephyrproject-rtos#6 0x7ff67b37b739 in cli_ui_out::do_message(ui_file_style const&, char const*, char*) C:/gdb/src/gdb.git/gdb/cli-out.c:227
    zephyrproject-rtos#7 0x7ff67ce3d030 in ui_out::call_do_message(ui_file_style const&, char const*, ...) C:/gdb/src/gdb.git/gdb/ui-out.c:571
    zephyrproject-rtos#8 0x7ff67ce4255a in ui_out::vmessage(ui_file_style const&, char const*, char*) C:/gdb/src/gdb.git/gdb/ui-out.c:740
    zephyrproject-rtos#9 0x7ff67ce2c873 in ui_file::vprintf(char const*, char*) C:/gdb/src/gdb.git/gdb/ui-file.c:73
    zephyrproject-rtos#10 0x7ff67ce7f83d in gdb_vprintf(ui_file*, char const*, char*) C:/gdb/src/gdb.git/gdb/utils.c:1881
    zephyrproject-rtos#11 0x7ff67ce7f83d in vwarning(char const*, char*) C:/gdb/src/gdb.git/gdb/utils.c:181
    zephyrproject-rtos#12 0x7ff67f3530eb in warning(char const*, ...) C:/gdb/src/gdb.git/gdbsupport/errors.cc:33
    zephyrproject-rtos#13 0x7ff67baed27f in gdb_bfd_close_warning C:/gdb/src/gdb.git/gdb/gdb_bfd.c:437
    zephyrproject-rtos#14 0x7ff67baed27f in gdb_bfd_close_or_warn C:/gdb/src/gdb.git/gdb/gdb_bfd.c:646
    zephyrproject-rtos#15 0x7ff67baed27f in gdb_bfd_unref(bfd*) C:/gdb/src/gdb.git/gdb/gdb_bfd.c:739
    zephyrproject-rtos#16 0x7ff68094b6f2 in gdb_bfd_ref_policy::decref(bfd*) C:/gdb/src/gdb.git/gdb/gdb_bfd.h:82
    zephyrproject-rtos#17 0x7ff68094b6f2 in gdb::ref_ptr<bfd, gdb_bfd_ref_policy>::~ref_ptr() C:/gdb/src/gdb.git/gdbsupport/gdb_ref_ptr.h:91
    zephyrproject-rtos#18 0x7ff67badf4d2 in gcore_command C:/gdb/src/gdb.git/gdb/gcore.c:176

0x1259ea503110 is located 16 bytes inside of 4064-byte region [0x1259ea503100,0x1259ea5040e0)
freed by thread T0 here:
    #0 0x7ff6806b1687 in free C:/gcc/src/gcc-14.2.0/libsanitizer/asan/asan_malloc_win.cpp:90
    zephyrproject-rtos#1 0x7ff67f2ae807 in objalloc_free C:/gdb/src/gdb.git/libiberty/objalloc.c:187
    zephyrproject-rtos#2 0x7ff67d7f56e3 in _bfd_free_cached_info C:/gdb/src/gdb.git/bfd/opncls.c:247
    zephyrproject-rtos#3 0x7ff67d7f2782 in _bfd_delete_bfd C:/gdb/src/gdb.git/bfd/opncls.c:180
    zephyrproject-rtos#4 0x7ff67d7f5df9 in bfd_close_all_done C:/gdb/src/gdb.git/bfd/opncls.c:960
    zephyrproject-rtos#5 0x7ff67d7f62ec in bfd_close C:/gdb/src/gdb.git/bfd/opncls.c:925
    zephyrproject-rtos#6 0x7ff67baecd27 in gdb_bfd_close_or_warn C:/gdb/src/gdb.git/gdb/gdb_bfd.c:643
    zephyrproject-rtos#7 0x7ff67baecd27 in gdb_bfd_unref(bfd*) C:/gdb/src/gdb.git/gdb/gdb_bfd.c:739
    zephyrproject-rtos#8 0x7ff68094b6f2 in gdb_bfd_ref_policy::decref(bfd*) C:/gdb/src/gdb.git/gdb/gdb_bfd.h:82
    zephyrproject-rtos#9 0x7ff68094b6f2 in gdb::ref_ptr<bfd, gdb_bfd_ref_policy>::~ref_ptr() C:/gdb/src/gdb.git/gdbsupport/gdb_ref_ptr.h:91
    zephyrproject-rtos#10 0x7ff67badf4d2 in gcore_command C:/gdb/src/gdb.git/gdb/gcore.c:176

It happens because gdb_bfd_close_or_warn uses a bfd-internal name for
the failing-close warning, after the close is finished, and the name
already freed:

static int
gdb_bfd_close_or_warn (struct bfd *abfd)
{
  int ret;
  const char *name = bfd_get_filename (abfd);

  for (asection *sect : gdb_bfd_sections (abfd))
    free_one_bfd_section (sect);

  ret = bfd_close (abfd);

  if (!ret)
    gdb_bfd_close_warning (name,
			   bfd_errmsg (bfd_get_error ()));

  return ret;
}

Fixed by making a copy of the name for the warning.

Approved-By: Andrew Burgess <aburgess@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants