Page MenuHomeFreeBSD

silence cast-align warnings from clang on powerpc64
ClosedPublic

Authored by alfredo.junior_eldorado.org.br on Jan 10 2019, 11:46 AM.

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
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 21953
Build 21193: arc lint + arc unit

Event Timeline

luporl added a subscriber: luporl.Jan 10 2019, 2:49 PM
emaste added inline comments.Jan 10 2019, 3:23 PM
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.

jhibbits accepted this revision.Jan 10 2019, 3:24 PM
This revision is now accepted and ready to land.Jan 10 2019, 3:24 PM
kib added a comment.Jan 10 2019, 3:37 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

kib added a comment.Jan 11 2019, 2:04 AM
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#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
kib accepted this revision.Jan 14 2019, 4:18 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)

bdragon accepted this revision.Jan 15 2019, 8:50 PM

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
luporl accepted this revision.Feb 13 2019, 6:22 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.