Sorry for the delay. I played a bit with emcripten to see if there was some misunderstanding going on, but it seems pretty clear that clang knows exactly what is being passed in via the va_arg call and just plain refuses to deal with a struct (at least for some architectures).
Consequently the original patch which restricts va_arg processing to simple types seems to me the most robust and portable solution. I will proceed with that for 0.7. Cliff a bit indicates to me that clang is not somehow confused about On Thu, Feb 27, 2014 at 9:01 AM, Fraser Adams <[email protected]> wrote: > Hi again Cliff, > Have you made any progress on this? > Cheers, > Frase > > > On 30/01/14 02:31, Cliff Jansen wrote: >> >> Well thanks for all that. I'll certainly take another look and report >> back before committing anything. Giving a compiler family indigestion >> is certainly to be avoided. >> >> Cliff >> >> On Tue, Jan 28, 2014 at 11:29 AM, Fraser Adams >> <[email protected]> wrote: >>> >>> Hi, me again Cliff. >>> I've only had time to recheck this against my test case, which is >>> representative of what I see for real. >>> >>> Doing: >>> >>> int pn_data_vfill2(const char *fmt, va_list ap) >>> { >>> // Process the PROPERTIES constant - this seems OK >>> uint64_t prop = va_arg(ap, uint64_t); >>> printf("prop = %llu\n", prop); >>> >>> { >>> pn_bytes_t bytes = va_arg(ap, pn_bytes_t); >>> printf("pn_data_vfill z, bytes.size = %zu, bytes.start = %s\n", >>> bytes.size, >>> bytes.start); >>> } >>> >>> // Process the char* returned by pn_string_get() >>> { >>> char *start = va_arg(ap, char *); >>> size_t size = strlen(start); >>> printf("pn_data_vfill size = %zu\n", size); >>> printf("pn_data_vfill string = %s\n", start); >>> } >>> >>> return 0; >>> } >>> >>> E.g. the both passing and retrieving structs approach of your second >>> approach actually doesn't even compile for me with LLVM le32, I get >>> >>> error: >>> cannot compile this aggregate va_arg expression yet >>> pn_bytes_t bytes = va_arg(ap, pn_bytes_t); >>> ^~~~~~~~~~~~~~~~~~~~~~ >>> /home/fadams/emscripten/system/include/libc/stdarg.h:15:25: note: >>> expanded >>> from >>> macro 'va_arg' >>> #define va_arg(v,l) __builtin_va_arg(v,l) >>> ^~~~~~~~~~~~~~~~~~~~~ >>> 1 error generated. >>> ERROR root: compiler frontend failed to generate LLVM bitcode, halting >>> make[2]: *** [CMakeFiles/test_varargs.dir/test_varargs.o] Error 1 >>> make[1]: *** [CMakeFiles/test_varargs.dir/all] Error 2 >>> make: *** [all] Error 2 >>> >>> Though it *does* work if I do "EMCC_LLVM_TARGET=i386-pc-linux-gnu make" >>> >>> >>> So in a nutshell: >>> 1) if I fudge the LLVM front end pure struct, put struct retrieve >>> discrete >>> and pure discrete values all work correctly. >>> >>> 2) with the standard emscripten le32 LLVM front end (which is what is >>> recommended) >>> * pure struct (as above) fails to compile >>> * put struct, retrieve discrete (as with the original Proton code) gives >>> the wrong results - that messed with my head :-) >>> * pure discrete values (as with your pn_string_size(msg->user_id), >>> pn_string_get(msg->user_id),) patch works correctly. >>> >>> >>> So I'd *really* like it if you could go down the path of your original >>> patch >>> 'cause that would fix what you've been seeing and let me compile the >>> JavaScript stuff without needing the environment tweaked (which is an >>> accident waiting to happen). I hope that you are agreeable to that? >>> >>> I think that it would probably be worth putting a comment in the call to >>> pn_data_fill and within the pn_data_vfill() 'z' case body to document >>> that >>> it's safer to pass individual entries to va_arg to avoid upsetting some >>> compilers - it's not entirely an *obvious* thing really :-D >>> >>> Cheers, >>> Frase >>> >>> >>> >>> >>> On 27/01/14 21:21, Cliff Jansen wrote: >>>> >>>> Thanks for the Javascript related info. >>>> >>>> Fraser: can you test if the review board patch (with the struct "in >>>> and out" strategy) works in your case with the unhacked llvm setup? >>>> If that works then I'll go ahead and check it in. >>>> >>>> If it fails, please try the first patch. If that works, we will just >>>> have to conclude that compilers have trouble with stucts in this case >>>> and fall back to passing the two basic types. That should be safe to >>>> work in the greatest number of cases. >>>> >>>> Many thanks. >>>> >>>> On Mon, Jan 27, 2014 at 1:13 PM, Fraser Adams (JIRA) <[email protected]> >>>> wrote: >>>>> >>>>> [ >>>>> >>>>> https://issues.apache.org/jira/browse/PROTON-488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13883314#comment-13883314 >>>>> ] >>>>> >>>>> Fraser Adams commented on PROTON-488: >>>>> ------------------------------------- >>>>> >>>>> Oh to be clear >>>>> either "struct in and out", or "separate size_t and char* in and out" >>>>> >>>>> what I meant in my previous comment was that the separate size_t and >>>>> char* in and out is what LLVM le32 is happiest with, the structs seem >>>>> to >>>>> confuse it, I hadn't realised that you'd changed your original patch to >>>>> now >>>>> do "all 'z' encodings to be passed as a single pn_bytes_t struct and >>>>> "retrieved" as a single pn_bytes_t struct" - although this arguably >>>>> looks >>>>> neater I'd definitely prefer your first approach. >>>>> >>>>>> Windows 7 64-bit VS2010 qpid-proton Crash on Startup with Send / Recv >>>>>> Application >>>>>> >>>>>> >>>>>> --------------------------------------------------------------------------------- >>>>>> >>>>>> Key: PROTON-488 >>>>>> URL: >>>>>> https://issues.apache.org/jira/browse/PROTON-488 >>>>>> Project: Qpid Proton >>>>>> Issue Type: Bug >>>>>> Components: proton-c >>>>>> Affects Versions: 0.6 >>>>>> Environment: Windows 7 64-bit VS 2010 >>>>>> Reporter: Frank Quinn >>>>>> Assignee: Cliff Jansen >>>>>> Priority: Critical >>>>>> Attachments: PROTON-488-0.patch, >>>>>> qpid-proton-win64-send-crash.png >>>>>> >>>>>> >>>>>> Steps to recreate: >>>>>> 1. Grab latest 0.6 tarball >>>>>> 2. Start up Visual Studio x64 Win64 Command Prompt (2010) and run >>>>>> "cmake >>>>>> ." to generate the visual studio files >>>>>> 3. Open Up the newly created Proton.sln in VS2010, right click on >>>>>> qpid-proton and add the path to python to executable directories >>>>>> 4. In the configuration manager, select qpid-proton and select active >>>>>> configuration to be Debug, then select "Add" to add x64 support, >>>>>> copying >>>>>> win32 configuration in the process. >>>>>> 5. Select qpid-proton properties and remove the hard coded >>>>>> /machine:X86 >>>>>> extra command lines in Linker -> Command Line (MACHINE:X64 should >>>>>> already be >>>>>> in the command line above so no need to add here) >>>>>> 6. Right click on qpid-proton and select build >>>>>> Repeat steps 3-6 for send / recv applications. When you run recv, then >>>>>> run send, you'll get a crash with the (soon to be attached) trace. >>>>>> Cheers, >>>>>> Frank >>>>> >>>>> >>>>> >>>>> -- >>>>> This message was sent by Atlassian JIRA >>>>> (v6.1.5#6160) >>> >>> >
