[PATCH v3 1/2] RISC-V: PR27916, Support mapping symbols.

[Nelson Chu]

Fixed the typo and comments, and the -D behavior of objdump (make it
same as aarch64 and arm).  Committed.

Thanks
Nelson

On Mon, Aug 30, 2021 at 11:32 AM Nelson Chu <nelson.chu@sifive.com> wrote:
>
> On Sat, Aug 28, 2021 at 11:47 AM Jim Wilson <jimw@sifive.com> wrote:
> >
> > On Tue, Aug 24, 2021 at 6:36 PM Nelson Chu <nelson.chu@sifive.com> wrote:
> >>
> >> ping :)
> >>
> >> Thanks
> >> Nelson
> >>
> >> On Tue, Jul 13, 2021 at 6:09 PM Nelson Chu <nelson.chu@sifive.com> wrote:
> >> >
> >> > Similar to ARM/AARCH64, we add mapping symbols in the symbol table,
> >> > to mark the start addresses of data and instructions.  The $d means
> >> > data, and the $x means instruction.  Then the disassembler uses these
> >> > symbols to decide whether we should dump data or instruction.
> >
> >
> > Looks OK to me.  I guess you want an approval because of the readelf.c change?  That is just a minor comment change though.
>
> Although I can approve my patch, and I should be able to handle most
> of the minor things, for some major changes and updates, I still hope
> that I can get some suggestions or approvals from other maintainers
> and experts.
>
> > Same file in riscv_mapping_state you have
> > +  /* The mapping symbol should be emitted.  */
> > +  if (from_state == to_state)
> > +    return;
> > which looks a little funny.  Maybe you meant that we should emit it now that we know that it is a text section.  But then you have code to not emit it if we are already in the right mapping state which is a little confusing.  Could be something like "should be emitted if not in the right mapping state."
>
> Oops, it should be "have been" rather than "be"...  For example, we
> already add the $d by the previous instruction, so we don't need to
> add a new $d if we still assemble instructions.  Yeah, "The mapping
> symbol should be emitted if not in the right mapping state.", it looks
> better, thanks.
>
> > I saw the comment about -D in the disassembler.  -D really is supposed to disassemble data.  The arm/aarch64 ports get this right.  -D is useful in rare cases where you have instruction templates in the data section, and you want to disassemble them to verify that they are correct.  Maybe you are disassembling a compiler so it has instructions in data.  Or maybe this is some self modifying code.  Anyways, -D should disassemble data same as arm/aarch64.
>
> OK I will fix this.
>
> > Mapping symbols are still a proposal for the psABI, but I think this is OK.  There was only disagreement on the more complicated stuff that this patch doesn't attempt to implement.
>
> Yeah, these complicated stuff, like $d<size> and $a, are deprecated
> now, arm/aarch64 already have been handling mapping symbols very well,
> it should be enough for us to follow their rules.
>
> Thanks
> Nelson