Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gcc13 warning: FFDC pointer alignment #1

Open
shenki opened this issue May 31, 2023 · 2 comments
Open

gcc13 warning: FFDC pointer alignment #1

shenki opened this issue May 31, 2023 · 2 comments

Comments

@shenki
Copy link
Member

shenki commented May 31, 2023

In file included from libekb.C:19:
libekb.C: In function 'void libekb_get_sbe_ffdc(FFDC&, const sbeFfdcPacketType&, int)':
./hwpf/fapi2/include/error_info/set_sbe_error.H:1493:86: warning: converting a packed 'fapi2::sbeFfdc_t' pointer (alignment 1) to a 'const uint32_t' {aka 'const unsigned int'} pointer (alignment 4) may result in an unaligned pointer value [-Waddress-of-packed-member]
 1493 |    fapi2::variable_buffer l_buffer((uint32_t*)FFDC_BUFFER, size_bytes/4, size_bytes*8);\
      |                                                                                      ^
libekb.C:258:9: note: in expansion of macro 'FAPI_SET_SBE_ERROR'
  258 |         FAPI_SET_SBE_ERROR(rc, ffdc_pkt.fapiRc, ffdc_endian.data(), proc_index);
      |         ^~~~~~~~~~~~~~~~~~
In file included from ./hwpf/fapi2/include/plat/plat_utils.H:27,
                 from libekb.C:11:
./ekb/hwpf/fapi2/include/error_info_defs.H:53:8: note: defined here
   53 | struct sbeFfdc_t
      |        ^~~~~~~~~
@shenki
Copy link
Member Author

shenki commented Jun 30, 2023

This code is generated by the perl script here:

print SBFILE " fapi2::variable_buffer l_buffer((uint32_t*)FFDC_BUFFER, size_bytes/4, size_bytes*8);\\\n";

The call site is here:

https://github.com/open-power/libekb_p10/blob/3d1b2963082d7725c3a4a2960d6f3f3336f401a7/libekb.C#L258

void libekb_get_sbe_ffdc(FFDC& ffdc, const sbeFfdcPacketType& ffdc_pkt,
                         int proc_index)
{
        using namespace fapi2;
        fapi2::ReturnCode rc;
        
        std::vector<sbeFfdc_t> ffdc_endian;

	FAPI_SET_SBE_ERROR(rc, ffdc_pkt.fapiRc, ffdc_endian.data(), proc_index);
typedef struct sbeFfdcPacket {
        uint32_t fapiRc;            // Fapi Rc
        uint32_t ffdcLengthInWords; // length of Fapi FFDC data
        uint32_t *ffdcData;         // fapi FFDC data
        ~sbeFfdcPacket()
        {
                // freeup the ffdcData allocated memory
                if (ffdcData) {
                        delete ffdcData;
                }
        }
} sbeFfdcPacketType;
struct sbeFfdc_t
{
    uint32_t  size;
    uint64_t  data;
} __attribute__((packed));
#define FAPI_SET_SBE_ERROR(RC,ERRVAL,FFDC_BUFFER,SBE_INSTANCE)\

FFDC_BUFFER is a ffdc_endian.data().

We are trying to take the data from the vector of struct sbeFfdc_t __attribute__(packed) and refer to it with a uint32_t *. Inside the variable_buffer constructor it inserts the data to bits_container iv_data.

typedef uint32_t container_unit;
typedef std::vector<container_unit> bits_container;

@shenki
Copy link
Member Author

shenki commented Jun 30, 2023

If I change the temporary buffer in libekb.C to a std::vector<uint32_t>:

--- a/libekb.C
+++ b/libekb.C
@@ -229,9 +229,9 @@ void libekb_get_sbe_ffdc(FFDC& ffdc, const sbeFfdcPacketType& ffdc_pkt,
        // Get size of FFDC data in bytes. exculde RC size.
        size_t size = (ffdc_pkt.ffdcLengthInWords - 1) * sizeof(uint32_t);
 
-       // Create temporary sbeFfdc_t to store the data after endianess
-       // conversion.
-       std::vector<sbeFfdc_t> ffdc_endian;
+       // Create temporary vector to store the data after endianess
+       // conversion. 
+       std::vector<uint32_t> ffdc_endian;
 
        // get size of sbeFfdc_t structre
        size_t sbe_ffdc_size = sizeof(sbeFfdc_t);
@@ -251,7 +251,8 @@ void libekb_get_sbe_ffdc(FFDC& ffdc, const sbeFfdcPacketType& ffdc_pkt,
                } else {
                        ffdc_data.data = sbe_ffdc->data;
                }
-               ffdc_endian.push_back(ffdc_data);
+               ffdc_endian.push_back(ffdc_data.size);
+               ffdc_endian.push_back(ffdc_data.data);

This breaks because the setSbeError calls in the generated hwpf/fapi2/include/error_info/set_sbe_error.H require a sbeFfdc_t.

I think the fix is to copy the data to an intermediate buffer before creating fapi2::variable_buffer l_buffer, or create a new fapi2::variable_buffer constructor that takes a byte array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant