[systemd-devel] compile with clang broken

[David Herrmann]

Hi

On Thu, Aug 14, 2014 at 8:07 PM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Fri, 18.07.14 16:02, Thomas H.P. Andersen (phomes at gmail.com) wrote:
>
>> 1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support
>> for looking up names) broke the build on clang.
>>
>> src/resolve/resolved-manager.c:553:43: error: non-const static data
>> member must be initialized out of line
>> uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct
>> in6_pktinfo)))
>>
>> Moving the MAX(...) to a separate line fixes that problem but another
>> error then happens:
>>
>> src/resolve/resolved-manager.c:554:25: error: fields must have a
>> constant size: 'variable length array in structure' extension will
>> never be supported
>> uint8_t buffer[CMSG_SPACE(size)
>>
>> We have encountered the same problem before and Lennart was able to
>> write the code in a different way. Would this be possible here too?
>
> My sugegstion here would be to maybe rewrite the MAX() macro to use
> __builtin_constant_p() so that it becomes constant if the params are
> constant, and only uses code block when it isn't. Or so...
>
> http://lists.freedesktop.org/archives/systemd-devel/2014-August/021912.html

Hm, I don't know whether that works. See the description here:
    https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html

What you propose is something like my attached patch, I guess? Along
the lines of:
    (__builtin_constant_p(A) && __builtin_constant_p(B)) ?
        ((A) > (B) ? (A) : (B)) :
        ({ ....OLD_MAX.... })

Thing is, the ELSE case is not considered a compile-time constant by
LLVM. Therefore, the whole expression is not considered a compile-time
constant. I don't know whether conditions with __builtin_constant_p()
are evaluated at the parser-step. The GCC example replaces the ELSE
case with -1, effectively making both compile-time constants.

I also added a test-case to make sure __builtin_constant_p doesn't
evaluate the arguments.

Can someone with LLVM set up give this a spin?

Thanks
David


diff --git a/src/shared/macro.h b/src/shared/macro.h
index 5619c32..18f5a79 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -140,6 +140,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) {
                         _a > _b ? _a : _b;       \
                 })

+#undef MAX
+#define MAX(_A, _B)                                                     \
+        __extension__ (                                                 \
+                (__builtin_constant_p(_A) && __builtin_constant_p(_B))  \
+                        ? (((_A) > (_B)) ? (_A) : (_B))                 \
+                        : ({                                            \
+                                typeof(_A) _a = (_A);                   \
+                                typeof(_B) _b = (_B);                   \
+                                _a > _b ? _a : _b;                      \
+                        }))
+
 #define MAX3(x,y,z)                              \
         __extension__ ({                         \
                         typeof(x) _c = MAX(x,y); \
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 1850f97..d348ac5 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -70,6 +70,20 @@ static void test_align_power2(void) {
         }
 }

+static void test_max(void) {
+        /* try triggering compile-errors for non-const initializers */
+        static const struct {
+                int a;
+        } val1 = {
+                .a = MAX(10, 100),
+        };
+        int d = 0;
+
+        assert_se(val1.a == 100);
+        assert_se(MAX(++d, 0) == 1);
+        assert_se(d == 1);
+}
+
 static void test_first_word(void) {
         assert_se(first_word("Hello", ""));
         assert_se(first_word("Hello", "Hello"));
@@ -926,6 +940,7 @@ int main(int argc, char *argv[]) {

         test_streq_ptr();
         test_align_power2();
+        test_max();
         test_first_word();
         test_close_many();
         test_parse_boolean();
-------------- next part --------------
A non-text attachment was scrubbed...
Name: max.patch
Type: text/x-patch
Size: 2008 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20140815/9fa07613/attachment.bin>