[PATCH] compiler-intel: fix wrong compiler barrier() macro

[PATCH] compiler-intel: fix wrong compiler barrier() macro

[Daniel Borkmann]

Cleanup commit 23ebdedc67e ("compiler-intel.h: Remove duplicate
definition") removed the double definition of __memory_barrier()
intrinsics.

However, in doing so, it also removed the preceding #undef barrier,
meaning, the actual barrier() macro from compiler-gcc.h with inline
asm is still in place when __GNUC__ is provided.

Subsequently, barrier() can never be defined as __memory_barrier()
from compiler.h since it already has a definition in place and if
we trust the comment in compiler-intel.h, ecc doesn't support gcc
specific asm statements. I don't have an ecc at hand, so a revert
of that cleanup would be the safest option, imho, as it has been
like this since pre-git times.

Fixes: 73679e508201 ("compiler-intel.h: Remove duplicate definition")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Pranith Kumar <bobby.prani@gmail.com>
Cc: H. Peter Anvin <hpa@linux.intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: mancha security <mancha1@zoho.com>
---
 include/linux/compiler-intel.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index ba147a1..5529c52 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -13,9 +13,12 @@
 /* Intel ECC compiler doesn't support gcc specific asm stmts.
  * It uses intrinsics to do the equivalent things.
  */
+#undef barrier
 #undef RELOC_HIDE
 #undef OPTIMIZER_HIDE_VAR
 
+#define barrier() __memory_barrier()
+
 #define RELOC_HIDE(ptr, off)					\
   ({ unsigned long __ptr;					\
      __ptr = (unsigned long) (ptr);				\
-- 
1.9.3

Re: [PATCH] compiler-intel: fix wrong compiler barrier() macro

[Pranith Kumar]

Hi Daniel,

On Wed, Apr 29, 2015 at 10:42 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Cleanup commit 23ebdedc67e ("compiler-intel.h: Remove duplicate
> definition") removed the double definition of __memory_barrier()
> intrinsics.
>
> However, in doing so, it also removed the preceding #undef barrier,
> meaning, the actual barrier() macro from compiler-gcc.h with inline
> asm is still in place when __GNUC__ is provided.

When you use the Intel compilers, the barrier() definition will come
from compiler.h and not compiler-gcc.h. That is what the commit
message says in 73679e508201(your commit message has the wrong hash).
I don't understand what problem you are seeing with this, can you
please explain?

Thanks!

Re: [PATCH] compiler-intel: fix wrong compiler barrier() macro

[mancha security]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

On Wed, Apr 29, 2015 at 10:51:40AM -0400, Pranith Kumar wrote:
> Hi Daniel,
> 
> On Wed, Apr 29, 2015 at 10:42 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> > Cleanup commit 23ebdedc67e ("compiler-intel.h: Remove duplicate
> > definition") removed the double definition of __memory_barrier()
> > intrinsics.
> >
> > However, in doing so, it also removed the preceding #undef barrier,
> > meaning, the actual barrier() macro from compiler-gcc.h with inline
> > asm is still in place when __GNUC__ is provided.
> 
> When you use the Intel compilers, the barrier() definition will come
> from compiler.h and not compiler-gcc.h. That is what the commit
> message says in 73679e508201(your commit message has the wrong hash).
> I don't understand what problem you are seeing with this, can you
> please explain?
> 
> Thanks!

Hi Pranith.

The problem is that ICC defines __GNUC__ so barrier() gets defined
in compiler-gcc.h. Your commit removed an #undef from compiler-intel.h
so compiler.h will never define barrier to __memory_barrier().

--mancha

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] compiler-intel: fix wrong compiler barrier() macro

[Pranith Kumar]

On Wed, Apr 29, 2015 at 10:59 AM, mancha security <mancha1@zoho.com> wrote:
>
> The problem is that ICC defines __GNUC__ so barrier() gets defined
> in compiler-gcc.h. Your commit removed an #undef from compiler-intel.h
> so compiler.h will never define barrier to __memory_barrier().
>

OK, I see your point. But, ICC has support for GCC inline assembly. So
the change does not seem to be making any difference. We are using our
own asm barrier rather than the inbuilt one provided by ICC.

-- 
Pranith

Re: [PATCH] compiler-intel: fix wrong compiler barrier() macro

[Daniel Borkmann]

On 04/29/2015 06:40 PM, Pranith Kumar wrote:
> On Wed, Apr 29, 2015 at 10:59 AM, mancha security <mancha1@zoho.com> wrote:
>>
>> The problem is that ICC defines __GNUC__ so barrier() gets defined
>> in compiler-gcc.h. Your commit removed an #undef from compiler-intel.h
>> so compiler.h will never define barrier to __memory_barrier().
>
> OK, I see your point. But, ICC has support for GCC inline assembly. So
> the change does not seem to be making any difference. We are using our
> own asm barrier rather than the inbuilt one provided by ICC.

It does make a difference: gcc inline assembly is not supported by
/ecc/, see that it's wrapped within the ifdef __ECC part. I believe,
that should be for ia64 which we have under arch/, no?

Re: [PATCH] compiler-intel: fix wrong compiler barrier() macro

[Pranith Kumar]

On Wed, Apr 29, 2015 at 12:59 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 04/29/2015 06:40 PM, Pranith Kumar wrote:
>>
>> On Wed, Apr 29, 2015 at 10:59 AM, mancha security <mancha1@zoho.com>
>> wrote:
>>>
>>>
>>> The problem is that ICC defines __GNUC__ so barrier() gets defined
>>> in compiler-gcc.h. Your commit removed an #undef from compiler-intel.h
>>> so compiler.h will never define barrier to __memory_barrier().
>>
>>
>> OK, I see your point. But, ICC has support for GCC inline assembly. So
>> the change does not seem to be making any difference. We are using our
>> own asm barrier rather than the inbuilt one provided by ICC.
>
>
> It does make a difference: gcc inline assembly is not supported by
> /ecc/, see that it's wrapped within the ifdef __ECC part. I believe,
> that should be for ia64 which we have under arch/, no?

Yes, looks like this breaks building the kernel with ecc compiler on
IA64. Has anyone tried building it with ECC on ia64 lately(or ever)?

Reviewed-by: Pranith Kumar <bobby.prani@gmail.com>

-- 
Pranith

Re: [PATCH] compiler-intel: fix wrong compiler barrier() macro

[mancha security]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

On Wed, Apr 29, 2015 at 12:40:15PM -0400, Pranith Kumar wrote:
> On Wed, Apr 29, 2015 at 10:59 AM, mancha security <mancha1@zoho.com>
> wrote:
> >
> > The problem is that ICC defines __GNUC__ so barrier() gets defined
> > in compiler-gcc.h. Your commit removed an #undef from
> > compiler-intel.h so compiler.h will never define barrier to
> > __memory_barrier().
> >
> 
> OK, I see your point. But, ICC has support for GCC inline assembly. So
> the change does not seem to be making any difference. We are using our
> own asm barrier rather than the inbuilt one provided by ICC.
> 
> -- Pranith

Yes, I misspoke earlier and meant to say ECC rather than ICC.

--mancha

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] compiler-intel: fix wrong compiler barrier() macro

[Daniel Borkmann]

On 04/29/2015 04:51 PM, Pranith Kumar wrote:
...
> message says in 73679e508201(your commit message has the wrong hash).

Sorry for that, the Fixes tag actually got it right.