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
<fraser.ad...@blueyonder.co.uk> 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) <j...@apache.org>
>> 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)
>
>
