[PATCH] RISC-V: Segment fault in riscv_elf_append_rela.

Nelson Chu nelson@rivosinc.com
Fri Mar 10 10:06:17 GMT 2023


Thanks!  Yeah I don't intend to fix the mismatch in the short-term, so
I will choose to use the solution 2, but report a dangerous relocation
for it.  But in the long-term, it would be great if the mismatch
problem can be resolved, so I will also add a comment to mention this
as a TODO.

Thanks again
Nelson

On Thu, Mar 9, 2023 at 8:59 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Thu, Mar 09, 2023 at 07:41:05PM +0800, Nelson Chu wrote:
> > Thanks for the suggestion :) Seems like there are two better solution for now,
> >
> > 1.  Add the BFD_ASSERT before calling every riscv_elf_append_rela to
> > make sure the written section exists, and then just return in the
> > riscv_elf_append_rela if the section doesn't exist, to avoid the
> > segment fault.  We will know which section is NULL, and where the
> > ASSERT code is.  Besides that, since parts of the callers already have
> > the BFD_ASSERT check before calling riscv_elf_append_rela, so this
> > seems the right way to do it.
> >
> > 2. Report error and return false if the section doesn't exist in the
> > riscv_elf_append_rela, and then the callers "goto out" to release the
> > unused data structures in the riscv_elf_relocate_section, or return
> > false if the caller is riscv_elf_finish_dynamic_symbol.  But it is
> > hard to report the more detailed errors in the riscv_elf_append_rela,
> > it is not easy to know which caller code meets the problem.
> >
> > I think solution 1 should be better than solution 2?
>
> Sorry, I should have read your email more carefully before commenting.
> I'm OK with an abort for things that can't happen.  ;-)  Now that I've
> read it again, I believe you are saying that there is currently a
> mismatch between check_relocs and relocate_section, and I'm assuming
> that you intend to fix that problem sometime.  If that is the case,
> then the abort in the original patch is OK.
>
> If you don't intend to fix the mismatch, *then* an abort is not OK and
> you should instead inform the user via an error message that his code
> is unsupported.
>
> >
> > Thanks
> > Nelson
> >
> > On Thu, Mar 9, 2023 at 4:34 PM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Thu, Mar 09, 2023 at 04:04:46PM +0800, Nelson Chu wrote:
> > > >     * elfnn-riscv.c (riscv_elf_append_rela): Abort when the dynamic section
> > > >     doesn't exist.
> > >
> > > It would be nicer to handle this more gracefully by returning an error
> > > to the caller.  An abort isn't much better than a segmentation fault
> > > if this can be triggered by user input.
> > >
> > > --
> > > Alan Modra
> > > Australia Development Lab, IBM
>
> --
> Alan Modra
> Australia Development Lab, IBM


More information about the Binutils mailing list