Skip to content

converter tool deletes variable arguments #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hasantouma opened this issue Jun 20, 2019 · 1 comment
Closed

converter tool deletes variable arguments #2

hasantouma opened this issue Jun 20, 2019 · 1 comment
Labels
bug Something isn't working

Comments

@hasantouma
Copy link
Collaborator

I noticed this issue when converting the Icecast code. In the file src/net/sock.c the function:
int sock_write(sock_t sock, const char *fmt, ...) is incorrectly converted to:
int sock_write(int sock, const char *fmt : itype(_Ptr<const char> ) )

I created a small example that highlights the issue:

Original C code var_arg.c:

#include <stdio.h>
#include <stdarg.h>

int fun(int a, ...) {
    // define type of variable
    va_list L;
    int z;

    z = 0;

    va_start(L, a);

    // Loop to adding the int values
    for (int i=0; i < a; i++) {
        z = z + va_arg(L, int);
    }

    va_end(L);

    return z;
}

int main() {
    // Define temporary variables
    int x, y, z;
    int k;

    x = 2;
    y = 3;
    z = 4;

    // calling function
    k = fun(3, x, y, z);

    // displaying message with result
    printf("Total of %d, %d, and %d is %d\n", x, y, z, k);

    return 0;
}

checked-c-convert output:
Note: I added #include <stdio_checked.h> and #include <stdchecked.h> before I ran the tool

#include <stdio_checked.h>
#include <stdarg.h>
#include <stdchecked.h>

int fun(int a) {
    // define type of variable
    va_list L;
    int z;

    z = 0;

    va_start(L, a);

    // Loop to adding the int values
    for (int i=0; i < a; i++) {
        z = z + va_arg(L, int);
    }

    va_end(L);

    return z;
}

int main() {
    // Define temporary variables
    int x, y, z;
    int k;

    x = 2;
    y = 3;
    z = 4;

    // calling function
    k = fun(3, x, y, z);

    // displaying message with result
    printf("Total of %d, %d, and %d is %d\n", x, y, z, k);

    return 0;
}

The tool deletes the variable argument in fun() causing an error when using va_start()

Machiry added a commit that referenced this issue Jul 11, 2019
@hasantouma hasantouma added the bug Something isn't working label Jul 12, 2019
@Machiry
Copy link
Collaborator

Machiry commented Jul 12, 2019

Fixed.

@Machiry Machiry closed this as completed Jul 12, 2019
rchyena pushed a commit that referenced this issue Jul 14, 2019
Machiry added a commit that referenced this issue Jul 26, 2019
Machiry pushed a commit that referenced this issue May 17, 2020
If a core file has an EFI version string which includes a UUID
(similar to what it returns for the kdp KDP_KERNELVERSION packet)
in the LC_IDENT or LC_NOTE 'kern ver str' load command.  In that
case, we should try to find the binary and dSYM for the UUID
listed.  The dSYM may have python code which knows how to relocate
the binary to the correct address in lldb's target section load
list and loads other ancillary binaries.

The test case is a little involved,

1. it compiles an inferior hello world apple (a.out),
2. it compiles a program which can create a corefile manually
   with a specific binary's UUID encoded in it,
3. it gets the UUID of the a.out binary,
4. it creates a shell script, dsym-for-uuid.sh, which will
   return the full path to the a.out + a.out.dSYM when called
   with teh correct UUID,
5. it sets the LLDB_APPLE_DSYMFORUUID_EXECUTABLE env var before
   creating the lldb target, to point to this dsym-for-uuid.sh,
6. runs the create-corefile binary we compiled in step #2,
7. loads the corefile from step #6 into lldb,
8. verifies that lldb loaded a.out by reading the LC_NOTE
   load command from the corefile, calling dsym-for-uuid.sh with
   that UUID, got back the path to a.out and loaded it.

whew!

<rdar://problem/47562911>

llvm-svn: 366378
@mwhicks1 mwhicks1 mentioned this issue Aug 19, 2020
mattmccutchen-cci pushed a commit that referenced this issue Feb 22, 2021
… Solaris/x86

A dozen 32-bit `AddressSanitizer` testcases FAIL on the latest beta of Solaris 11.4/x86, e.g.
`AddressSanitizer-i386-sunos :: TestCases/null_deref.cpp` produces

  AddressSanitizer:DEADLYSIGNAL
  =================================================================
  ==29274==ERROR: AddressSanitizer: stack-overflow on address 0x00000028 (pc 0x08135efd bp 0xfeffdfd8 sp 0x00000000 T0)
      #0 0x8135efd in NullDeref(int*) /vol/llvm/src/llvm-project/dist/compiler-rt/test/asan/TestCases/null_deref.cpp:15:10
      #1 0x8135ea6 in main /vol/llvm/src/llvm-project/dist/compiler-rt/test/asan/TestCases/null_deref.cpp:21:3
      #2 0x8084b85 in _start (null_deref.cpp.tmp+0x8084b85)

   SUMMARY: AddressSanitizer: stack-overflow /vol/llvm/src/llvm-project/dist/compiler-rt/test/asan/TestCases/null_deref.cpp:15:10 in NullDeref(int*)
  ==29274==ABORTING

instead of the expected

  AddressSanitizer:DEADLYSIGNAL
  =================================================================
  ==29276==ERROR: AddressSanitizer: SEGV on unknown address 0x00000028 (pc 0x08135f1f bp 0xfeffdf48 sp 0xfeffdf40 T0)
  ==29276==The signal is caused by a WRITE memory access.
  ==29276==Hint: address points to the zero page.
      #0 0x8135f1f in NullDeref(int*) /vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/null_deref.cpp:15:10
      #1 0x8135efa in main /vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/null_deref.cpp:21:3
      #2 0x8084be5 in _start (null_deref.cpp.tmp+0x8084be5)

  AddressSanitizer can not provide additional info.
   SUMMARY: AddressSanitizer: SEGV /vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/null_deref.cpp:15:10 in NullDeref(int*)
  ==29276==ABORTING

I managed to trace this to a change in `<sys/regset.h>`: previously the header would
primarily define the short register indices (like `UESP`). While they are required by the
i386 psABI, they are only required in `<ucontext.h>` and could previously leak into
unsuspecting user code, polluting the namespace and requiring elaborate workarounds
like that in `llvm/include/llvm/Support/Solaris/sys/regset.h`. The change fixed that by restricting
the definition of the short forms appropriately, at the same time defining all `REG_` prefixed
forms for compatiblity with other systems.  This exposed a bug in `compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp`, however:
Previously, the index for the user stack pointer would be hardcoded if `REG_ESP`
wasn't defined. Now with that definition present, it turned out that `REG_ESP` was the wrong index to use: the previous value 17 (and `REG_SP`) corresponds to `REG_UESP`
instead.

With that change, the failures are all gone.

Tested on `amd-pc-solaris2.11`.

Differential Revision: https://reviews.llvm.org/D83664
mattmccutchen-cci pushed a commit that referenced this issue Aug 3, 2021
… disabled for a PCH vs a module file

This addresses an issue with how the PCH preable works, specifically:

1. When using a PCH/preamble the module hash changes and a different cache directory is used
2. When the preamble is used, PCH & PCM validation is disabled.

Due to combination of #1 and #2, reparsing with preamble enabled can end up loading a stale module file before a header change and using it without updating it because validation is disabled and it doesn’t check that the header has changed and the module file is out-of-date.

rdar://72611253

Differential Revision: https://reviews.llvm.org/D95159
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants