Page MenuHomeFreeBSD

silence cast-align warnings from clang on powerpc64
ClosedPublic

Authored by alfredo on Jan 10 2019, 11:46 AM.
Tags
None
Referenced Files
F102916361: D18807.id52827.diff
Mon, Nov 18, 5:30 PM
Unknown Object (File)
Fri, Nov 15, 2:45 AM
Unknown Object (File)
Wed, Nov 13, 6:15 PM
Unknown Object (File)
Sat, Nov 9, 2:47 AM
Unknown Object (File)
Fri, Nov 8, 4:27 AM
Unknown Object (File)
Thu, Nov 7, 8:32 PM
Unknown Object (File)
Thu, Nov 7, 1:33 PM
Unknown Object (File)
Wed, Nov 6, 9:06 PM

Details

Summary

silence the following warning when compiling libthr with clang 8 for powerpc64 architecture:

usr/src/lib/libthr/arch/powerpc/include/pthread_md.h:82:10: error: cast from 'uint8_t *' (aka 'unsigned char *') to 'struct tcb *'

increases required alignment from 1 to 8 [-Werror,-Wcast-align]
  return ((struct tcb *)(_tp - TP_OFFSET));
Test Plan

build base, then chroot to new sysroot and run:

on powerpc64

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lib/libthr/Makefile
20–22 ↗(On Diff #52727)

I think it would be good to also include a comment about why we know the alignment is not a practical issue.

This revision is now accepted and ready to land.Jan 10 2019, 3:24 PM

So it is only in _tcb_get() ? Can you use #pragma to only disable the warning at the place ? https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas
I do not like too broad settings which might prevent some useful warning ignored.

In D18807#401468, @kib wrote:

So it is only in _tcb_get() ? Can you use #pragma to only disable the warning at the place ? https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas
I do not like too broad settings which might prevent some useful warning ignored.

I agree, it would be nice to have it limited to the actual cast. Combined with perhaps asserting the alignment of the result manually.

C question: Would a cast through void* be a better workaround for this specific issue by the way?

In D18807#401486, @git_bdragon.rtk0.net wrote:
In D18807#401468, @kib wrote:

So it is only in _tcb_get() ? Can you use #pragma to only disable the warning at the place ? https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas
I do not like too broad settings which might prevent some useful warning ignored.

I agree, it would be nice to have it limited to the actual cast. Combined with perhaps asserting the alignment of the result manually.

Can we assume it's safe to ignore it in the .c is safe for all platforms in any compiler? (actually just for any powerpc* for this copy of pthreads_md.h)

There's another "broader platform" warning that apears if I remove "CFLAGS.thr_stack.c+= -Wno-cast-align" from this Makefile

/usr/src/lib/libthr/thread/thr_stack.c:303:17: error: cast from 'char *' to 'struct stack *' increases required alignment from 1 to 8

[-Werror,-Wcast-align]
          spare_stack = (struct stack *)
lib/libthr/Makefile
20–22 ↗(On Diff #52727)

Actually I'm not sure if it's not an issue. I was assuming it's not because this Makefile already contains the following lines:

NO_WCAST_ALIGN.gcc=1 # for gcc 4.2
CFLAGS.thr_stack.c+= -Wno-cast-align

In D18807#401490, @git_bdragon.rtk0.net wrote:

C question: Would a cast through void* be a better workaround for this specific issue by the way?

Might be if it works, but I am not sure what compiler authors think about void * alignment requirements. So any way to silence the warning, but make it local to this place.

In D18807#401500, @alfredo.junior_eldorado.org.br wrote:
In D18807#401486, @git_bdragon.rtk0.net wrote:
In D18807#401468, @kib wrote:

So it is only in _tcb_get() ? Can you use #pragma to only disable the warning at the place ? https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas
I do not like too broad settings which might prevent some useful warning ignored.

I agree, it would be nice to have it limited to the actual cast. Combined with perhaps asserting the alignment of the result manually.

Can we assume it's safe to ignore it in the .c is safe for all platforms in any compiler? (actually just for any powerpc* for this copy of pthreads_md.h)

There are pragma push/pop ops which should allow you to localize the ignore made with pragmas.

There's another "broader platform" warning that apears if I remove "CFLAGS.thr_stack.c+= -Wno-cast-align" from this Makefile

/usr/src/lib/libthr/thread/thr_stack.c:303:17: error: cast from 'char *' to 'struct stack *' increases required alignment from 1 to 8

[-Werror,-Wcast-align]
          spare_stack = (struct stack *)

It is of similar nature, I believe: the correctly aligned pointer is stored in char *.

dropped libthr Makefile change and adjusted __tcb_get asm to decrement pointer instead

This revision now requires review to proceed.Jan 14 2019, 4:04 PM
This revision is now accepted and ready to land.Jan 14 2019, 4:18 PM

The new version of the patch isn't ignoring or working around compiler warning, it does pointer arithmetic in asm instead, based on idea from git_bdragon.rtk0.net (thanks!).

Tested on PowerPC64 using attached test code. (I cannot test on 32 bit at this moment)

32 bit version works fine on G4.

move the "-" signal to input value making it easyer to read

This revision now requires review to proceed.Jan 16 2019, 12:14 PM

Approving the minor '-' signal move to unblock this change.

This revision is now accepted and ready to land.Feb 13 2019, 6:22 PM
This revision was automatically updated to reflect the committed changes.