default alignment?

In Sema::ActOnPragmaPack, I see this:

  // FIXME: This should come from the target.
  if (AlignmentVal == 0)
    AlignmentVal = 8;

So, what's the right fix? Should this be
(Context.Target.getDoubleAlign() / Context.Target.getCharWidth())? (And
why 8? Where does that come from?)

Hi Eric,

I'm not familiar with #pragma pack show, but the 8 here is in bits. The right fix is probably to call Context.Target.getCharWidth().

-Chris

Hi Chris,

That's just can't be right. A default alignment of 1 byte would lead to
alignment faults all over the place. And consider the following:

#pragma pack(show)
#pragma pack(1)
#pragma pack(show)

Compiled with MSVC, I get:

foo.c(1) : warning C4810: value of pragma pack(show) == 8
foo.c(3) : warning C4810: value of pragma pack(show) == 1

Clearly, the "1" in the last line doesn't refer to a 1 *bit* alignment.
We're talking bytes here.

In Sema::ActOnPragmaPack, I see this:

// FIXME: This should come from the target.
if (AlignmentVal == 0)
   AlignmentVal = 8;

So, what's the right fix? Should this be
(Context.Target.getDoubleAlign() / Context.Target.getCharWidth())? (And
why 8? Where does that come from?)

Hi Eric,

I'm not familiar with #pragma pack show, but the 8 here is in bits. The right fix is probably to call Context.Target.getCharWidth().

Hi Chris,

That's just can't be right.

It is in bytes, see
http://gcc.gnu.org/onlinedocs/gcc/Structure_002dPacking-Pragmas.html

A default alignment of 1 byte would lead to
alignment faults all over the place.

Actually an alignment of 1 byte means that none of your struct elements
are aligned, and that the compiler should emit byte-level reads
even for larger fields on architectures that don't support unaligned
accesses.

Best regards,
--Edwin

As I said, I don't know anything about pack(show). The context of the code is:

  case Sema::PPK_Show: // pack(show)
    // Show the current alignment, making sure to show the right value
    // for the default.
    AlignmentVal = Context->getAlignment();
    // FIXME: This should come from the target.
    if (AlignmentVal == 0)
      AlignmentVal = 8;
    if (AlignmentVal == PackStackEntry::kMac68kAlignmentSentinel)
      Diag(PragmaLoc, diag::warn_pragma_pack_show) << "mac68k";
    else
      Diag(PragmaLoc, diag::warn_pragma_pack_show) << AlignmentVal;
    break;

Since "Context->getAlignment()" returns alignment in bits, it looks like show is just broken. It should be something like:

...
      Diag(PragmaLoc, diag::warn_pragma_pack_show) << AlignmentVal/8;
    break;

-Chris

In Sema::ActOnPragmaPack, I see this:

// FIXME: This should come from the target.
if (AlignmentVal == 0)
  AlignmentVal = 8;

So, what's the right fix? Should this be
(Context.Target.getDoubleAlign() / Context.Target.getCharWidth())? (And
why 8? Where does that come from?)

Hi Eric,

I'm not familiar with #pragma pack show, but the 8 here is in bits. The right fix is probably to call Context.Target.getCharWidth().

Hi Chris,

That's just can't be right. A default alignment of 1 byte would lead to
alignment faults all over the place. And consider the following:

#pragma pack(show)
#pragma pack(1)
#pragma pack(show)

Compiled with MSVC, I get:

foo.c(1) : warning C4810: value of pragma pack(show) == 8
foo.c(3) : warning C4810: value of pragma pack(show) == 1

Clearly, the "1" in the last line doesn't refer to a 1 *bit* alignment.
We're talking bytes here.

As I said, I don't know anything about pack(show). The context of the code is:

  case Sema::PPK_Show: // pack(show)
    // Show the current alignment, making sure to show the right value
    // for the default.
    AlignmentVal = Context->getAlignment();
    // FIXME: This should come from the target.
    if (AlignmentVal == 0)
      AlignmentVal = 8;
    if (AlignmentVal == PackStackEntry::kMac68kAlignmentSentinel)
      Diag(PragmaLoc, diag::warn_pragma_pack_show) << "mac68k";
    else
      Diag(PragmaLoc, diag::warn_pragma_pack_show) << AlignmentVal;
    break;

Since "Context->getAlignment()" returns alignment in bits, it looks like show is just broken. It should be something like:

...
      Diag(PragmaLoc, diag::warn_pragma_pack_show) << AlignmentVal/8;
    break;

I think you're mistaken. Context->getAlignment() appears to be returning
alignment in bytes, not bits, and pragma pack(show) appears to be
working just fine. This is from test/Sema/pragma-pack.c:

#pragma pack(1)
#pragma pack(push, 8) // -> (eek, 2), (, 2), (, 1), 8
/* expected-warning {{value of #pragma pack(show) == 8}} */ #pragma
pack(show)
#pragma pack(pop) // -> (eek, 2), (,2), 1
/* expected-warning {{value of #pragma pack(show) == 1}} */ #pragma
pack(show)

Notice that we can pack(1) (1-byte alignment) and then pack(show)
displays 1.

Getting back to my original question:

So, what's the right fix? Should this be
(Context.Target.getDoubleAlign() / Context.Target.getCharWidth())?
(And why 8? Where does that come from?)

Any help would be much appreciated.