On Wed, Aug 5, 2020 at 11:53 AM Richard Biener <rguent...@suse.de> wrote: > > On August 5, 2020 4:45:00 PM GMT+02:00, "H.J. Lu" <hjl.to...@gmail.com> wrote: > >On Wed, Aug 5, 2020 at 5:34 AM H.J. Lu <hjl.to...@gmail.com> wrote: > >> > >> On Wed, Aug 5, 2020 at 5:31 AM Richard Biener <rguent...@suse.de> > >wrote: > >> > > >> > On Wed, 5 Aug 2020, H.J. Lu wrote: > >> > > >> > > On Wed, Aug 5, 2020 at 12:06 AM Richard Biener > ><rguent...@suse.de> wrote: > >> > > > > >> > > > On Tue, 4 Aug 2020, H.J. Lu wrote: > >> > > > > >> > > > > On Tue, Aug 4, 2020 at 12:35 AM Richard Biener > ><rguent...@suse.de> wrote: > >> > > > > > > >> > > > > > On Mon, 3 Aug 2020, Qing Zhao wrote: > >> > > > > > > >> > > > > > > Hi, Uros, > >> > > > > > > > >> > > > > > > Thanks a lot for your review on X86 parts. > >> > > > > > > > >> > > > > > > Hi, Richard, > >> > > > > > > > >> > > > > > > Could you please take a look at the middle-end part to > >see whether the > >> > > > > > > rewritten addressed your previous concern? > >> > > > > > > >> > > > > > I have a few comments below - I'm not sure I'm qualified to > >fully > >> > > > > > review the rest though. > >> > > > > > > >> > > > > > > Thanks a lot. > >> > > > > > > > >> > > > > > > Qing > >> > > > > > > > >> > > > > > > > >> > > > > > > > On Jul 31, 2020, at 12:57 PM, Uros Bizjak > ><ubiz...@gmail.com> wrote: > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > 22:05, tor., 28. jul. 2020 je oseba Qing Zhao > ><qing.z...@oracle.com <mailto:qing.z...@oracle.com>> napisala: > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > Richard and Uros, > >> > > > > > > > > > >> > > > > > > > > Could you please review the change that H.J and I > >rewrote based on your comments in the previous round of discussion? > >> > > > > > > > > > >> > > > > > > > > This patch is a nice security enhancement for GCC > >that has been requested by security people for quite some time. > >> > > > > > > > > > >> > > > > > > > > Thanks a lot for your time. > >> > > > > > > > > >> > > > > > > > I'll be away from the keyboard for the next week, but > >the patch needs a middle end approval first. > >> > > > > > > > > >> > > > > > > > That said, x86 parts looks OK. > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > Uros. > >> > > > > > > > > Qing > >> > > > > > > > > > >> > > > > > > > > > On Jul 14, 2020, at 9:45 AM, Qing Zhao via > >Gcc-patches <gcc-patches@gcc.gnu.org <mailto:gcc-patches@gcc.gnu.org>> > >wrote: > >> > > > > > > > > > > >> > > > > > > > > > Hi, Gcc team, > >> > > > > > > > > > > >> > > > > > > > > > This patch is a follow-up on the previous patch and > >corresponding discussion: > >> > > > > > > > > > > >https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545101.html > ><https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545101.html> > ><https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545101.html > ><https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545101.html>> > >> > > > > > > > > > > >> > > > > > > > > > From the previous round of discussion, the major > >issues raised were: > >> > > > > > > > > > > >> > > > > > > > > > A. should be rewritten by using regsets > >infrastructure. > >> > > > > > > > > > B. Put the patch into middle-end instead of x86 > >backend. > >> > > > > > > > > > > >> > > > > > > > > > This new patch is rewritten based on the above 2 > >comments. The major changes compared to the previous patch are: > >> > > > > > > > > > > >> > > > > > > > > > 1. Change the names of the option and attribute > >from > >> > > > > > > > > > > >-mzero-caller-saved-regs=[skip|used-gpr|all-gpr|used|all] and > >zero_caller_saved_regs("skip|used-gpr|all-gpr||used|all”) > >> > > > > > > > > > to: > >> > > > > > > > > > > >-fzero-call-used-regs=[skip|used-gpr|all-gpr|used|all] and > >zero_call_used_regs("skip|used-gpr|all-gpr||used|all”) > >> > > > > > > > > > Add the new option and new attribute in general. > >> > > > > > > > > > 2. The main code generation part is moved from i386 > >backend to middle-end; > >> > > > > > > > > > 3. Add 4 target-hooks; > >> > > > > > > > > > 4. Implement these 4 target-hooks on i386 backend. > >> > > > > > > > > > 5. On a target that does not implement the target > >hook, issue error for the new option, issue warning for the new > >attribute. > >> > > > > > > > > > > >> > > > > > > > > > The patch is as following: > >> > > > > > > > > > > >> > > > > > > > > > [PATCH] Add > >-fzero-call-used-regs=[skip|used-gpr|all-gpr|used|all] > >> > > > > > > > > > command-line option and > >> > > > > > > > > > > >zero_call_used_regs("skip|used-gpr|all-gpr||used|all") function > >attribue: > >> > > > > > > > > > > >> > > > > > > > > > 1. -fzero-call-used-regs=skip and > >zero_call_used_regs("skip") > >> > > > > > > > > > > >> > > > > > > > > > Don't zero call-used registers upon function > >return. > >> > > > > > > >> > > > > > Does a return via EH unwinding also constitute a function > >return? I > >> > > > > > think you may want to have a finally handler or support in > >the unwinder > >> > > > > > for this? Then there's abnormal return via longjmp & > >friends, I guess > >> > > > > > there's nothing that can be done there besides patching > >glibc? > >> > > > > > >> > > > > Abnormal returns, like EH unwinding and longjmp, aren't > >covered by this > >> > > > > patch. Only normal returns are covered. > >> > > > > >> > > > What's the point then? Also specifically thinking about spill > >slots. > >> > > > > >> > > > >> > > The goal of this patch is to zero caller-saved registers upon > >normal > >> > > function return. Abnormal returns and spill slots are outside of > >the > >> > > scope of this patch. > >> > > >> > Sure, I can write a patch that spills some regs, writes zeros to > >them > >> > and then restores them. And the patch will fulfil what it was > >designed > >> > to do. > >> > > >> > Still I need to come up with a reason that this is a useful feature > >> > by its own for it to be accepted. > >> > > >> > I am asking for that reason. What's the reason for the "goal of > >this > >> > patch"? Why's that a useful goal on its own? > >> > > >> > >> Hi Victor, > >> > >> Can you provide some background information about how/why this > >feature > >> is used? > >> > > > >From The SECURE project and GCC in GCC Cauldron 2018: > > > >Speaker: Graham Markall > > > >The SECURE project is a 15 month program funded by Innovate UK, to > >take well known security techniques from academia and make them > >generally available in standard compilers, specfically GCC and LLVM. > >An explicit objective is for those techniques to be incorporated in > >the upstream versions of compilers. The Cauldron takes place in the > >final month of the project and this talk will present the technical > >details of some of the techniques implemented, and review those that > >are yet to be implemented. A particular focus of this talk will be on > >verifying that the implemetnation is correct, which can be a bigger > >challenge than the implementation. > > > >Techniques to be covered in the project include the following: > > > >Stack and register erasure. Ensuring that on return from a function, > >no data is left lying on the stack or in registers. Particular > >challenges are in dealing with inlining, shrink wrapping and caching. > > > >This patch implemens register erasure. > > Part of it, yes. While I can see abnormal transfer of control is difficult > exception handling is used too wide spread to be ignored. What's the plan > there?
The initial usage is in Linux kernel where user space EH isn't an issue. Further improvement can be investigated later. > So can we also see the other parts? In particular I wonder whether exposing > just register clearing (in this fine-grained manner) is required and useful > rather than thinking of a better interface for the whole thing? > > Richard. This patch is for caller-saved registers only. Stack temporaries aren't covered by this. We can simply clear the stack first before releasing it for function return. -- H.J.
