marler8997 / ziglibc

193 stars 19 forks source link

test abi sizes #1

Open matu3ba opened 2 years ago

matu3ba commented 2 years ago

Zig ensures types have properly defined sizes. C does not. For example, char is implementation defined and typically signed. However, it also can be unsigned on some targets.

Practical example: Your implementation of of strlen uses

export fn strlen(s: [*:0]const u8) callconv(.C) usize {
    trace.log("strlen {}", .{trace.fmtStr(s)});
    const result = std.mem.len(s);
    trace.log("strlen return {}", .{result});
    return result;
}

However, you export size_t strlen(const char *s);, which does not ensure the behavior.

  1. Thus I would propose to ensure via macros that users have correct behavior, until ziglibc has all C type shennanigans which can be switched on via macros to include the desired functions.
  2. Or, properly better to communicate clearly to users what stuff to include for checking that behavior is as expected.

Macros for usage

// MACROS
#define IS_SIGNED(Type) (((Type)-1) < 0)

#include <assert.h>
static_assert(sizeof(U8) == 1, "U8 must have 1 byte size"); // compile-time error

// http://scaryreasoner.wordpress.com/2009/02/28/checking-sizeof-at-compile-time/
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
usage: BUILD_BUG_ON( sizeof(someThing) != PAGE_SIZE );

A generalization can be applied to the other type sizes.

marler8997 commented 2 years ago

whoa I've never seen static_assert before. I've actually already added a runtime time for size_t and ssize_t here: https://github.com/marler8997/ziglibc/blob/main/test/types.c

This test will currently get run when you run zig build test.

matu3ba commented 2 years ago

Related to this would be breaking abi changes, ie from Microsoft that can be tested with this tool: https://github.com/lvc/abi-compliance-checker from this dedicated website http://abi-laboratory.pro/

Related stackoverflow question: https://stackoverflow.com/questions/9233118/library-abi-compatibility-between-versions-of-visual-studio

This would go into the direction of automatic generation of macro includes + header to include as checks for the caller site.

Ideally, there would be macro aliasing with a reasonable compatibility symbol, but I have no idea how big of a maintenance mess this would be.

matu3ba commented 2 years ago

One working version for me without the sizeof stuff (C11+ compatible):

#include <stdint.h> // uint32_t, uint8_t
#include <stdlib.h> // exit
#include <stdio.h>  // fprintf
#ifndef __cplusplus // disable clang complains
#ifdef TRUE
#error "TRUE already defined"
#else
#define TRUE (1==1)
#endif

#ifdef FALSE
#error "False already defined"
#else
#define FALSE (!TRUE)
#endif

// existence of typedefs can not be checked within macros
//#define _TYPEDEF_
typedef enum { false = FALSE, true } bool;
//#endif

// potential necessity: custom printf and exit for the platform
// unfortunately we dont have __COLUMN__ as macro
#define assert(a) if( !( a ) )                            \
{                                                         \
    fprintf( stderr, "%s:%d assertion failure of (%s)\n", \
                             __FILE__, __LINE__, #a );    \
    exit( 1 );                                            \
}

#ifdef static_assert
#error "static_assert already defined"
#else
#define static_assert _Static_assert // since C11
#endif

#ifdef IS_SIGNED
#error "IS_SIGNED already defined"
#else
#define IS_SIGNED(Type) (((Type)-1) < 0)
#endif

//static_assert(IS_SIGNED(char) == false, "char is signed");    // ERRORS on x86_64 Linux (glibc/musl)
static_assert(IS_SIGNED(char), "char is unsigned");    // NO ERROR on x86_64 Linux (glibc/musl)
#endif // __cplusplus

A rather hacky, but very efficient+compact+lazy, way to generate the headers/write the sizes for stdint.h is used in skalibs:

src/include/$(package)/uint64.h: $(sysdeps)/sysdeps src/headers/bits-header src/headers/bits-footer src/headers/bits-lendian src/headers/bits-bendian src/headers/bits-template src/headers/uint64-ulong64 src/headers/uint64-noulong64 src/headers/uint64-defs src/headers/uint64-macros
    exec tools/gen-bits.sh $(sysdeps)/sysdeps 64 21 25 17 65 > $@
matu3ba commented 2 years ago

update of script, which just works on a toy project https://github.com/matu3ba/pbcd/blob/master/common.h Sorry for noise and please let me know, what stuff you see as useful + where so I can PR it.

#ifndef __cplusplus // disable clang complains
#include <stdint.h> // abi: uint32_t, uint8_t
#include <stdlib.h> // assert: exit
#include <stdio.h>  // assert: fprintf

#ifdef TRUE
#error "TRUE already defined"
#else
#define TRUE (1==1)
#endif

#ifdef FALSE
#error "False already defined"
#else
#define FALSE (!TRUE)
#endif

// existence of typedefs can not be checked within macros
//#define _TYPEDEF_
typedef enum { false = FALSE, true } bool;
//#endif

#ifdef static_assert
#error "static_assert already defined"
#else
#define static_assert _Static_assert // since C11
#endif

// potential necessity: custom printf and exit for the platform
// unfortunately we dont have __COLUMN__ as macro
#define assert(a) if( !( a ) )                            \
{                                                         \
    fprintf( stderr, "%s:%d assertion failure of (%s)\n", \
                             __FILE__, __LINE__, #a );    \
    exit( 1 );                                            \
}                                                         \
static_assert(true, "")

#ifdef IS_SIGNED
#error "IS_SIGNED already defined"
#else
#define IS_SIGNED(Type) (((Type)-1) < 0)
#endif

static_assert(IS_SIGNED(char),   "err: char is unsigned");
static_assert(sizeof(char) == 1, "err: char not 1 byte");
static_assert(sizeof(unsigned char) == 1, "err: char not 1 byte");
static_assert(sizeof(signed char) == 1, "err: char not 1 byte");
static_assert(sizeof(uint8_t) == 1,  "err: uint8_t not 1 byte");
static_assert(sizeof(uint16_t) == 2, "err: uint16_t not 2 byte");
static_assert(sizeof(uint32_t) == 4, "err: uint32_t not 4 byte");
static_assert(sizeof(uint64_t) == 8, "err: uint64_t not 8 byte");
static_assert(sizeof(int8_t) == 1,  "err: int8_t not 1 byte");
static_assert(sizeof(int16_t) == 2, "err: int16_t not 2 byte");
static_assert(sizeof(int32_t) == 4, "err: int32_t not 4 byte");
static_assert(sizeof(int64_t) == 8, "err: int64_t not 8 byte");
//static_assert(sizeof(uint128_t) == 16, "err: uint128_t not 16 byte"); poorly supported
//static_assert(sizeof(int128_t) == 16, "err: int128_t not 16 byte"); poorly supported
#endif // __cplusplus
ikskuh commented 1 year ago

Please do not define the reserved bool type yourself, but include <stdbool.h> which is a compiler-provided builtin since C99 that provides proper platform compatibility.

Also asserting the sizeof(char) is a bit pointless, as sizeof(char) is defined the be 1.

matu3ba commented 1 year ago

Please do not define the reserved bool type yourself, but include which is a compiler-provided builtin since C99 that provides proper platform compatibility.

Thats true, but the user can override this via external code (from a different compiler/vendor) for size reduction, so it is no hard guarantee (so strictly speaking requires additional checks) https://stackoverflow.com/a/4446591/9306292. Typical examples are DSPs, which tend to be word-addressable via 4 bytes and one can reduce this to 1 byte. There should be an explicit check to make clear, that this is not a supported use case.

Other than that

#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

can be assumed. for C99. ansi c has no stdint making it wild. see also https://en.cppreference.com/w/c/header.

Also asserting the sizeof(char) is a bit pointless, as sizeof(char) is defined the be 1.

You are correct, I messed this up with CHAR_BIT context.

matu3ba commented 1 year ago

Just to keep in mind, if you every choose to add assertions for typecheck and want to remain c++ compatible: _Static_assert is not compatible with C++ and neither is generic selection (C11 feature) to typecheck C code with C++ compatible:

// C11's Generic selection: Return constant 1, if type and 0 otherwise
// Keep things clean via stringification
#define STATIC_ASSERT_H(x)  _Static_assert(x, #x)
#define STATIC_ASSERT(x)    STATIC_ASSERT_H(x)
#define OBJ_IS_OF_TYPE(Type, Obj) _Generic(Obj, Type: 1, default: 0)
// Note: C++11 has other utilities and is incompatible with _Static_assert
// #include <type_traits>
// #define OBJ_IS_OF_TYPE(Type, Obj) std::is_same<decltype(Obj), Type>::value>
// #define STATIC_ASSERT(Input) static_assert(Input)
// { Usage
//     #include <time.h>
//     STATIC_ASSERT(OBJ_IS_OF_TYPE(timespecval1.tv_sec, int64_t));
//     STATIC_ASSERT(OBJ_IS_OF_TYPE(timespecval1.tv_nsec, int64_t));
// }

// Hygienic macros
#define TEST_FUNC(a,b)                                         \
do {                                                           \
    STATIC_ASSERT(OBJ_IS_OF_TYPE(a, int64_t));                 \
    STATIC_ASSERT(OBJ_IS_OF_TYPE(b, int64_t));                 \
} while(0)