Skip to content
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

Feature request: require explicit casting of any **pointer to void* #1940

Closed
alexveden opened this issue Feb 8, 2025 · 4 comments
Closed
Labels
Discussion needed This feature needs discussion to iron out details

Comments

@alexveden
Copy link
Contributor

I love writing C, with its all problems, I find them manageable. However, there is one issue which costs me hours of stupid debugging, of code which looks valid and just yesterday passed all tests in the place which today is segfaulting.

The symptoms of this issue are so wild and unpredictable, and may expose themselves in segfaulting, weird value printing, or cascading unpredictable behavior of the all sorts of code which rely on call with this issue. C compiler is never complaining about them, sanitizers are calm, and debuggers show just in the structure data.

Problem definition

I'm talking about passing a pointer of a pointer to a void* parameters. Usually it happens after refactoring, where initially we have a struct, which may be initialized locally in the function, but then we decided to refactor the code and pass a pointer to this structure.

import std::io;

struct Foo
{
    int bar;
}

fn void main()
{
    io::printfn("hello");
    foo();
    Foo f = { .bar = 7 };
    foo_refactored(&f);
}

fn void foo()
{
    Foo f = { .bar = 3 };
    print_foo(&f);
}

fn void foo_refactored(Foo* f)
{
    print_foo(&f);
}

fn void print_foo(void* fptr)
{
    Foo* f = fptr;
    io::printfn("foo.bar = %s", f.bar);
}

Output is:

hello
foo.bar = 3
foo.bar = -340106060

So in the case of my program, it didn't segfaulted, and just printed data. Which is more insane to debug than segfault, because the behavior of the program may become completely unpredictable in absolutely different place.

How c3 is affected

Initially, I caught this problem when I was refactoring std::io::path::PathWalker function (same refactoring issue, as above). However, it's very popular with C interop and threads too. Issue is getting worse when we didn't have source code of receiving side of a call (e.g., libc).

test/test_suite/methods/method_from_var.c3|1 col 1| def NodeNotifyHandler = fn void(TreeView* this, TreeNode* node, String prop, void* data);
lib/std/io/os/file_nolibc.c3|4 col 1| def FopenFn = fn void*!(String, String);
lib/std/io/os/file_nolibc.c3|5 col 1| def FreopenFn = fn void*!(void*, String, String);
lib/std/io/os/file_nolibc.c3|6 col 1| def FcloseFn = fn void!(void*);
lib/std/io/os/file_nolibc.c3|7 col 1| def FseekFn = fn void!(void*, isz, Seek);
lib/std/io/os/file_nolibc.c3|8 col 1| def FtellFn = fn usz!(void*);
lib/std/io/os/file_nolibc.c3|9 col 1| def FwriteFn = fn usz!(void*, char[] buffer);
lib/std/io/os/file_nolibc.c3|10 col 1| def FreadFn = fn usz!(void*, char[] buffer);
lib/std/io/os/file_nolibc.c3|12 col 1| def FputcFn = fn void!(int, void*);
lib/std/io/path.c3|544 col 1| def PathWalker = fn bool! (Path, bool is_dir, void*);
lib/std/io/formatter.c3|23 col 1| def OutputFn = fn void!(void* buffer, char c);
lib/std/libc/os/posix.c3|32 col 1| def SigActionFunction =  fn void(CInt, void*, void*);
lib/std/threads/thread.c3|20 col 1| def ThreadFn = fn int(void* arg);
lib/std/os/posix/process.c3|59 col 1| def BacktraceFn = fn CInt(void** buffer, CInt size);
lib/std/os/macos/objc.c3|8 col 1| def SendVoid = fn void*(void*, ObjcSelector);
lib/std/os/posix/threads.c3|9 col 1| def PosixThreadFn = fn void*(void*);
lib/std/libc/libc.c3|35 col 1| def CompareFunction = fn int(void*, void*);

Proposed solution

  1. I think it would be very beneficial to compiler check if pointer which passed to void* is not a double pointer, and if it's the case, the compiler should force explicit cast.
fn void foo_refactored(Foo* f)
{
    print_foo(&f);   // <<--- compilation error
    // You are passing Foo** to void*, which is unsusual, use explicit cast (Foo**) if you intended to use double pointer.

    // no compilation error, but at least it highlights the problem, if the programmer decided to make shortcut
    //    or maybe didn't properly understand the problem at all
    print_foo((Foo**)&f);   
}
  1. For higher level C3 functions (at least I see PathWalk and ThreadFn as good candidates) maybe it would be safer to switch to any instead of void, and let user do explicit anycast with type check?
@Book-reader
Copy link
Contributor

wouldn't requiring print_foo((void*)&f) make more sense than print_foo((Foo**)&f)?

@alexveden
Copy link
Contributor Author

wouldn't requiring print_foo((void*)&f) make more sense than print_foo((Foo**)&f)?

Imagine two internal dialogues of a person who would have to debug this:

Case A:

  • OK, print_foo expects void* so I send it &f which has to be a pointer
  • Why it's requiring (Foo**) or why print_foo((Foo**)&f) is cast to double pointer, I need just Foo* ?
  • This is weird, why &f is a double pointer? Oops, print_foo(f); is valid, because f is a Foo*

Case B:

  • OK, print_foo expects void* so I send it &f which has to be a pointer
  • Why it's requiring (void*) cast? It must be done automatically?
  • print_foo((void*)&f) is legit call, the problem has to be in different place

@lerno
Copy link
Collaborator

lerno commented Feb 8, 2025

It's something that could be added in obnoxious mode perhaps.

@lerno lerno added the Discussion needed This feature needs discussion to iron out details label Feb 9, 2025
@alexveden
Copy link
Contributor Author

@lerno can you recall from your practice any cases when we need to pass a typed double pointer to void* argument of a function?

In my mind, it could be passing char** argv into thread function, maybe. But in C3 we 99% of a time would pass String[]* argv, the same with arrays and lists. In C array pointers would be usually int**, but in C3 highly likely int[]*.

I've just grepped all stdlib for ** combination, and there are only ~100 cases of use of double pointers and ** in comments in C3 stdlib. So most of the time, ** is a typed function argument. And about 20 cases of use void** which is explicitly a double pointer.

So with this reasoning, possibly it is worth thinking about adding this in normal mode?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion needed This feature needs discussion to iron out details
Projects
None yet
Development

No branches or pull requests

3 participants