Build failed on riscv64

yuzibo commented 1 month ago

Hi, On my local build on riscv64 hardware, I got:

murmurhash3.c:48:29: error: cast increases required alignment of target type [-Werror=cast-align]
   48 |         const u64 *blocks = (const u64 *)(data);
      |                             ^
if cmp -s .deps/io-factory.d .deps/io-factory.d.new; then \

I think this is useful to generate such an error on riscv64 according to Instruction Length Encoding section of RISCV spec. So we have to deal with the code more carefully.

Please correct me if I am wrong.

slegendr commented 1 month ago

Hello,

Thank you for bringing this to our attention. Unfortunately, we do not support or test against the riscv64 architecture internally. Could you please provide more information on your set up and the build/test procedures you were executing so that I can investigate further? Additionally, if you have any suggestions on how to go about emulating a RISC-V environment, I'd appreciate them.

Thanks!

yuzibo commented 1 month ago

Thanks for your response.:)

The error is easy to reproduce on riscv64 hardware, just make it and then we can get the error.

In fact, the root cause is not from vdo's side. It is more inclined to the riscv64 gcc does not like cast-align warning and we targeted the warning as error. There is no any warnings on x86 with the same code.

From my side, may you can try it on docker riscv64/debian and I think this is the most conventional. If you have any trouble to run the code on riscv64, please feel free to tell me.

code to reproduce:

int main() {
        const uint8_t *key;

        const uint64_t *blocks = (const uint64_t *)(key);

        return 0;
}
# gcc -g test.c -o test -Wcast-align -Werror

slegendr commented 1 month ago

Okay, we have some ideas on how to correct this that I'm exploring. Once I get something put together and working internally, would you be willing to test in on your hardware? That would speed up the process significantly, compared to getting a riscv64 container set up.

fvcr commented 1 month ago

Hello everyone,

I'm the current maintainer of vdo on Debian. In case it helps, you can check out the build logs we have at Buildd status for vdo (sid).

We are successfully building on the architectures:

amd64
arm64
ppc64el
s390x
loong64
ppc64

The first upload was made on August 22, 2024: Accepted vdo 8.3.0-71-1exp1 into experimental.

However, it failed to compile for the following architectures:

armel:

dm-bufio.c:156:73: note: format string is defined here
156 |                                          "error reading physical page %lu",
  |                                                                       ~~^
  |                                                                         |
  |                                                                         long unsigned int
  |                                                                       %llu
cc1: all warnings being treated as errors
make[3]: *** [Makefile:123: fileUtils.o] Error 1

armhf:

./linux/cache.h:21:2: error: #error "unknown cache line size"
21 | #error "unknown cache line size"
  |  ^~~~~
cc1: all warnings being treated as errors
In file included from ./linux/cache.h:11:
funnel-queue.h:56:18: error: ‘L1_CACHE_BYTES’ undeclared here (not in a function)
56 | struct __aligned(L1_CACHE_BYTES) funnel_queue {
  |                  ^~~~~~~~~~~~~~
./linux/compiler_attributes.h:17:52: note: in definition of macro ‘__aligned’
17 | #define __aligned(x)    __attribute__((__aligned__(x)))
  |                                                    ^
make[3]: *** [Makefile:123: dm-bufio.o] Error 1
In file included from memory-alloc.h:9,
             from fileUtils.c:16:
./linux/cache.h:21:2: error: #error "unknown cache line size"
21 | #error "unknown cache line size"
  |  ^~~~~
memory-alloc.h: In function ‘vdo_allocate_cache_aligned’:
memory-alloc.h:112:42: error: ‘L1_CACHE_BYTES’ undeclared (first use in this function)
112 |         return vdo_allocate_memory(size, L1_CACHE_BYTES, what, ptr);
  |                                          ^~~~~~~~~~~~~~
memory-alloc.h:112:42: note: each undeclared identifier is reported only once for each function it appears in
make[3]: *** [Makefile:123: funnel-queue.o] Error 1
make[3]: *** [Makefile:123: delta-index.o] Error 1
make[3]: *** [Makefile:123: fileUtils.o] Error 1

i386:

logger.h:43:71: note: in definition of macro ‘vdo_log_strerror’
43 |         __vdo_log_strerror(priority, errnum, VDO_LOGGING_MODULE_NAME, __VA_ARGS__)
  |                                                                       ^~~~~~~~~~~
dm-bufio.c:155:17: note: in expansion of macro ‘vdo_log_warning_strerror’
155 |                 vdo_log_warning_strerror(result,
  |                 ^~~~~~~~~~~~~~~~~~~~~~~~
dm-bufio.c:156:73: note: format string is defined here
156 |                                          "error reading physical page %lu",
  |                                                                       ~~^
  |                                                                         |
  |                                                                         long unsigned int
  |                                                                       %llu
cc1: all warnings being treated as errors

mips64el:

memory-alloc.h:112:42: note: each undeclared identifier is reported only once for each function it appears in
In file included from thread-utils.h:9,
             from dm-bufio.c:14:
./linux/atomic.h: In function ‘smp_mb’:
./linux/atomic.h:102:2: error: #error "no fence defined"
102 | #error "no fence defined"
  |  ^~~~~
./linux/atomic.h: In function ‘smp_rmb’:
./linux/atomic.h:131:2: error: #error "no fence defined"
131 | #error "no fence defined"
  |  ^~~~~
./linux/atomic.h: In function ‘smp_wmb’:
./linux/atomic.h:160:2: error: #error "no fence defined"
160 | #error "no fence defined"
  |  ^~~~~
make[3]: *** [Makefile:123: dm-bufio.o] Error 1

riscv64:

murmurhash3.c: In function ‘murmurhash3_128’:
murmurhash3.c:48:29: error: cast increases required alignment of target type [-Werror=cast-align]
48 |         const u64 *blocks = (const u64 *)(data);
  |                             ^
cc1: all warnings being treated as errors
make[3]: *** [Makefile:123: murmurhash3.o] Error 1

alpha:

./linux/atomic.h: In function ‘smp_mb’:
./linux/atomic.h:102:2: error: #error "no fence defined"
102 | #error "no fence defined"
  |  ^~~~~
./linux/atomic.h: In function ‘smp_rmb’:
./linux/atomic.h:131:2: error: #error "no fence defined"
131 | #error "no fence defined"
  |  ^~~~~
./linux/atomic.h: In function ‘smp_wmb’:
./linux/atomic.h:160:2: error: #error "no fence defined"
160 | #error "no fence defined"
  |  ^~~~~
In file included from ./linux/cache.h:11:
funnel-queue.h: At top level:
funnel-queue.h:56:18: error: ‘L1_CACHE_BYTES’ undeclared here (not in a function)
56 | struct __aligned(L1_CACHE_BYTES) funnel_queue {
  |                  ^~~~~~~~~~~~~~
./linux/compiler_attributes.h:17:52: note: in definition of macro ‘__aligned’
17 | #define __aligned(x)    __attribute__((__aligned__(x)))
  |                                                    ^
make[3]: *** [Makefile:123: config.o] Error 1

hppa:

dm-bufio.c:155:17: note: in expansion of macro ‘vdo_log_warning_strerror’
155 |                 vdo_log_warning_strerror(result,
  |                 ^~~~~~~~~~~~~~~~~~~~~~~~
make[3]: *** [Makefile:123: chapter-index.o] Error 1
make[3]: *** Waiting for unfinished jobs....
dm-bufio.c:156:73: note: format string is defined here
156 |                                          "error reading physical page %lu",
  |                                                                       ~~^
  |                                                                         |
  |                                                                         long unsigned int
  |                                                                       %llu
cc1: all warnings being treated as errors

m68k:

funnel-queue.h: At top level:
funnel-queue.h:56:18: error: ‘L1_CACHE_BYTES’ undeclared here (not in a function)
56 | struct __aligned(L1_CACHE_BYTES) funnel_queue {
  |                  ^~~~~~~~~~~~~~
./linux/compiler_attributes.h:17:52: note: in definition of macro ‘__aligned’
17 | #define __aligned(x)    __attribute__((__aligned__(x)))
  |                                                    ^
In file included from ./linux/compiler.h:9,
             from ./errors.h:9,
             from ./thread-utils.h:15:
funnel-queue.h:56:18: error: ‘L1_CACHE_BYTES’ undeclared here (not in a function)
56 | struct __aligned(L1_CACHE_BYTES) funnel_queue {
  |                  ^~~~~~~~~~~~~~
./linux/compiler_attributes.h:17:52: note: in definition of macro ‘__aligned’
17 | #define __aligned(x)    __attribute__((__aligned__(x)))
  |                                                    ^
make[3]: *** [Makefile:124: chapter-index.o] Error 1

powerpc:

dm-bufio.c:143:40: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘sector_t’ {aka ‘long long unsigned int’} [-Werror=format=]
143 |                                        "error reading physical page %lu",
  |                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
144 |                                        block);
  |                                        ~~~~~
  |                                        |
  |                                        sector_t {aka long long unsigned int}
logger.h:43:71: note: in definition of macro ‘vdo_log_strerror’
43 |         __vdo_log_strerror(priority, errnum, VDO_LOGGING_MODULE_NAME, __VA_ARGS__)
  |                                                                       ^~~~~~~~~~~
dm-bufio.c:142:17: note: in expansion of macro ‘vdo_log_error_strerror’
142 |                 vdo_log_error_strerror(-PTR_ERR(data),
  |                 ^~~~~~~~~~~~~~~~~~~~~~
dm-bufio.c:143:71: note: format string is defined here
143 |                                        "error reading physical page %lu",
  |                                                                     ~~^
  |                                                                       |
  |                                                                       long unsigned int
  |                                                                     %llu
dm-bufio.c:156:42: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘sector_t’ {aka ‘long long unsigned int’} [-Werror=format=]
156 |                                          "error reading physical page %lu",
  |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
157 |                                          block);
  |                                          ~~~~~
  |                                          |
  |                                          sector_t {aka long long unsigned int}
logger.h:43:71: note: in definition of macro ‘vdo_log_strerror’
43 |         __vdo_log_strerror(priority, errnum, VDO_LOGGING_MODULE_NAME, __VA_ARGS__)
  |                                                                       ^~~~~~~~~~~
dm-bufio.c:155:17: note: in expansion of macro ‘vdo_log_warning_strerror’
155 |                 vdo_log_warning_strerror(result,
  |                 ^~~~~~~~~~~~~~~~~~~~~~~~
dm-bufio.c:156:73: note: format string is defined here
156 |                                          "error reading physical page %lu",
  |                                                                       ~~^
  |                                                                         |
  |                                                                         long unsigned int
  |                                                                       %llu
if cmp -s .deps/funnel-queue.d .deps/funnel-queue.d.new; then \

sh4:

./linux/compiler_attributes.h:17:52: note: in definition of macro ‘__aligned’
17 | #define __aligned(x)    __attribute__((__aligned__(x)))
  |                                                    ^
In file included from funnel-queue.h:10,
             from indexer.h:18:
./linux/cache.h: At top level:
./linux/cache.h:21:2: error: #error "unknown cache line size"
21 | #error "unknown cache line size"
  |  ^~~~~
In file included from ./linux/compiler.h:9,
             from ./errors.h:9,
             from ./thread-utils.h:15:
funnel-queue.h:56:18: error: ‘L1_CACHE_BYTES’ undeclared here (not in a function)
56 | struct __aligned(L1_CACHE_BYTES) funnel_queue {
  |                  ^~~~~~~~~~~~~~
./linux/compiler_attributes.h:17:52: note: in definition of macro ‘__aligned’
17 | #define __aligned(x)    __attribute__((__aligned__(x)))
  |                                                    ^
make[3]: *** [Makefile:124: chapter-index.o] Error 1

sparc64:


./linux/atomic.h: In function ‘smp_rmb’:
./linux/atomic.h:131:2: error: #error "no fence defined"
131 | #error "no fence defined"
  |  ^~~~~
./linux/atomic.h: In function ‘smp_wmb’:
./linux/atomic.h:160:2: error: #error "no fence defined"
160 | #error "no fence defined"
  |  ^~~~~
In file included from index.c:10:
memory-alloc.h: In function ‘vdo_allocate_cache_aligned’:
memory-alloc.h:113:1: error: control reaches end of non-void function [-Werror=return-type]
113 | }
  | ^
make[3]: *** [Makefile:124: chapter-index.o] Error 1

- x32:

dm-bufio.c:156:73: note: format string is defined here 156	"error reading physical page %lu",	~~^
long unsigned int
%llu

cc1: all warnings being treated as errors make[3]: *** [Makefile:123: dm-bufio.o] Error 1



I hope this helps in some way.

Cheers.

yuzibo commented 1 month ago

Okay, we have some ideas on how to correct this that I'm exploring. Once I get something put together and working internally, would you be willing to test in on your hardware? That would speed up the process significantly, compared to getting a riscv64 container set up.

No problem. Once we are ready to test some code, you can ping me at any time.:)

yuzibo commented 1 month ago

Hello everyone,

I'm the current maintainer of vdo on Debian. In case it helps, you can check out the build logs we have at Buildd status for vdo (sid).

We are successfully building on the architectures:

amd64

arm64

ppc64el

s390x

loong64

ppc64

In my view, armel,i386, hppa, riscv64 and x32 was failed due to cc1: all warnings being treated as errors. In other words, warning from compiler treated as error. I did not think vdo was ready to support other architectures from the additional error log.

slegendr commented 1 month ago

Hello,

What I am working on for this issue is code cleanup in our upstream project (vdo-devel), as there is no need to require u64 alignment in this part of the code. The changes would be available in this repo with the next release of the vdo user tools.

As for the build errors for various architectures noted earlier, it is important to consider the following:

VDO does not support or work on 32-bit architectures, so building against them should be avoided.
Using VDO on small devices doesn't make sense, so building against architectures intended for small devices or embedded processors is also not recommended, even if they are 64-bit.

Although we don't intend to support these architectures officially since we cannot readily test against them, we welcome patch submissions. For the user tools specifically, patches can be submitted to this project, or our vdo-devel project. For kernel code changes, patches can be submitted upstream to dm-devel.

slegendr commented 3 weeks ago

Hello @yuzibo - I submitted the change in our upstream project and it has been merged.

If you would like to test building with the changes, you can apply the changes in this diff to utils/uds/murmurhash3.c in your clone of this repo and build the package following the README instructions.

Please let me know if the issue can be closed. Thanks!

yuzibo commented 3 weeks ago

@slegendr Thanks for working on this.

I try it with your patch, hmm, but unfortunately, it go further but still failed with the same reason:

...
    -c -MD -MF .deps/requestQueue.d.new -MP -MT requestQueue.o requestQueue.c -o requestQueue.o
In file included from ./errors.h:9,
                 from ./thread-utils.h:15,
                 from ./linux/mutex.h:12,
                 from indexer.h:9,
                 from funnel-requestqueue.h:9,
                 from requestQueue.c:6:
requestQueue.c: In function ‘poll_queues’:
./linux/compiler.h:28:17: error: cast increases required alignment of target type [-Werror=cast-align]
   28 |                 (type *) ((char *) __mptr - offsetof(type, member)); \
      |                 ^
requestQueue.c:142:24: note: in expansion of macro ‘container_of’
  142 |                 return container_of(entry, struct uds_request, queue_link);
      |                        ^~~~~~~~~~~~
./linux/compiler.h:28:17: error: cast increases required alignment of target type [-Werror=cast-align]
   28 |                 (type *) ((char *) __mptr - offsetof(type, member)); \
      |                 ^
requestQueue.c:146:24: note: in expansion of macro ‘container_of’
  146 |                 return container_of(entry, struct uds_request, queue_link);
      |                        ^~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [Makefile:124: requestQueue.o] Error 1
make[2]: Leaving directory '/home/rv/git/vdo/vdo/utils/uds'
make[1]: *** [Makefile:24: all] Error 1
make[1]: Leaving directory '/home/rv/git/vdo/vdo/utils'
make: *** [Makefile:32: all] Error 1
...

Ping me at any time if you need more test.

slegendr commented 3 weeks ago

It is good that it got past the original issue and the cleanup was successful. Since the original issue is resolved, I am going to close this issue.

For any remaining compiler errors related to cast alignment, I suggest that you submit a PR that resolves them to either this repo, or vdo-devel. That would be best, as we cannot reproduce them internally since the riscv64 architecture isn't officially supported.

raeburn commented 3 weeks ago

Just a comment on the container_of alignment issue: As in the kernel, container_of is used when we have a pointer to a field of a structure and want a pointer to the containing structure. The onus of ensuring that the value is a field in a structure of a certain type and not a random separately-allocated object of that type is on the surrounding code; it may mean, for example, that a callback function knows that it's only called from a function using pointers to members of certain structure types.

So, in general, I'm not sure there is a good way to alter the code to make such alignment clear to the compiler. (We might need to say, for example, "this points inside a structure that is 16-byte aligned, to a field that is offset by a multiple of 16 bytes plus an extra 4 bytes".) If the container_of macro can be modified to suppress that diagnostic in just those invocations, that might be best. Otherwise, perhaps the alignment check should be disabled.

dm-vdo / vdo

Build failed on riscv64 #69