EliaGeretto wrote:
> > > I almost did not notice this patch as I am not added a reviewer. The
> > > clang change makes sense but can you extract the lld/ELF to a separate
> > > patch?
> > >
> > > * lld: Add -z memtag-{mode,heap,stack}
> > > * clang: Switch -fsanitize=memtag to use -z memtag-* linker flags
> >
> >
> > Sorry, I don't think I can add reviewers directly, I think I can just tag
> > people in comments.
> > Anyway, two separate commits was my original plan, but @fmayer requested
> > them to be merged together in a single one. The reason is that the lld
> > patch breaks backwards compatibility by removing some flags, so we would
> > have a commit with the clang driver which is incompatible with lld. That
> > would also probably break some of the clang tests for memtag. In addition,
> > if we split in two commits and only one gets reverted, the incompatiblity
> > would pop up again; reverting as a single unit is better. This reasoning
> > makes sense to me, @MaskRay do you still want them separate?
>
> They should be separate in principle. I see that you removed options from
> `lld/ELF/Options.td` - which is acceptable for fr this Android feature and
> Android bundles clang and lld. So I think it's fine for this specific case.
Ok, I will split the change then. I will tag you in the two new PRs.
Unfortunately, I cannot easily stack them with Graphite, not having commit
access, but I will mark them as one depending on the other.
https://github.com/llvm/llvm-project/pull/187055
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
