On Tue, Nov 7, 2017 at 4:29 PM, Atish Patra <atish.pa...@oracle.com> wrote: > On 11/07/2017 04:42 PM, Y Song wrote: >> >> On Tue, Nov 7, 2017 at 2:04 PM, Alexei Starovoitov <a...@fb.com> wrote: >>> >>> On 11/8/17 6:47 AM, Y Song wrote: >>>> >>>> >>>> On Tue, Nov 7, 2017 at 1:39 PM, Alexei Starovoitov <a...@fb.com> wrote: >>>>> >>>>> >>>>> On 11/8/17 6:14 AM, Y Song wrote: >>>>>> >>>>>> >>>>>> >>>>>> On Tue, Nov 7, 2017 at 12:37 AM, Naveen N. Rao >>>>>> <naveen.n....@linux.vnet.ibm.com> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> Alexei Starovoitov wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 11/7/17 12:55 AM, Naveen N. Rao wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> I thought such struct shouldn't change layout. >>>>>>>>>> If it is we need to fix include/linux/compiler-clang.h to do that >>>>>>>>>> anon struct as well. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> We considered that, but it looked to be very dependent on the >>>>>>>>> version >>>>>>>>> of >>>>>>>>> gcc used to build the kernel. But, this may be a simpler approach >>>>>>>>> for >>>>>>>>> the shorter term. >>>>>>>>> >>>>>>>> >>>>>>>> why it would depend on version of gcc? >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> From what I can see, randomized_struct_fields_start is defined only >>>>>>> for >>>>>>> gcc >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> = 4.6. For older versions, it does not get mapped to an anonymous >>>>>>> >>>>>>> >>>>>>> >>>>>>> structure. We may not care for older gcc versions, but.. >>>>>>> >>>>>>> The other issue was that __randomize_layout maps to __designated_init >>>>>>> when >>>>>>> randstruct plugin is not enabled, which is in turn an attribute on >>>>>>> gcc >>>>>>>> >>>>>>>> = >>>>>>> >>>>>>> v5.1, but not otherwise. >>>>>>> >>>>>>>> We just need this, no? >>>>>>>> >>>>>>>> diff --git a/include/linux/compiler-clang.h >>>>>>>> b/include/linux/compiler-clang.h >>>>>>>> index de179993e039..4e29ab6187cb 100644 >>>>>>>> --- a/include/linux/compiler-clang.h >>>>>>>> +++ b/include/linux/compiler-clang.h >>>>>>>> @@ -15,3 +15,6 @@ >>>>>>>> * with any version that can compile the kernel >>>>>>>> */ >>>>>>>> #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), >>>>>>>> __COUNTER__) >>>>>>>> + >>>>>>>> +#define randomized_struct_fields_start struct { >>>>>>>> +#define randomized_struct_fields_end }; >>>>>>>> >>>>>>>> since offsets are mandated by C standard. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Yes, this is what we're testing with and is probably sufficient for >>>>>>> our >>>>>>> purposes. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Just tested this with bcc. bcc actually complains. the rewriter >>>>>> is not able to rewrite prev->pid where prev is "struct task_struct >>>>>> *prev". >>>>>> I will change bcc rewriter to see whether the field value is correct >>>>>> or >>>>>> not. >>>>>> >>>>>> Not sure my understanding is correct or not, but I am afraid that >>>>>> the above approach for clang compiler change may not work. >>>>>> If clang calculates the field offset based on header file, the offset >>>>>> may not be the same as kernel one.... >>>>> >>>>> >>>>> >>>>> >>>>> why is that? >>>>> When randomization is off both gcc and clang must generate the same >>>>> offsets, since it's C standard. >>>> >>>> >>>> >>>> The patch changed compiler-clang.h, so gcc still do randomization. >>> >>> >>> >>> gcc_plugins are off by default and randomization will not be >>> turned on for any sane distro or datacenter that cares about >>> performance and stability. >>> So imo above compiler-clang.h patch together with bcc fix would >>> be enough. >> >> >> Agree that short time the suggested fix should be enough. >> Long time, disto could become "insane" someday :-) >> > > Yup it works with clang compiler & bcc hack. Thanks :). > I verified it with task_switch.py & runqlat.py. > > Are you going to push the bcc hacks to github repo ?
I will need to have proper implementation than a hack. Yes, once done, will push into bcc repo. Should be done soon. > > Regards, > Atish