0

The following method is causing bad memory allocation exceptions, I can't seem to find where this is exactly happening, for context the method is using the isl library to construct a map containing a set of iterators names with their number of constraints:


    std::unordered_map<std::string, int> utility::get_constraints_map(isl_set *set)
    {
    isl_basic_set_list *bset_list = isl_set_get_basic_set_list(set);
    std::unordered_map<std::string, int> constraints_map{};
    
    int n_basic_set = isl_set_n_basic_set(set);
    for (int i = 0; i < n_basic_set; i++)
    {
        isl_basic_set *bset = isl_basic_set_list_get_basic_set(bset_list, i);
        isl_constraint_list *cst_list = isl_basic_set_get_constraint_list(bset);
        for (int j = 0; j < isl_constraint_list_n_constraint(cst_list); j++)
        {
            isl_constraint *cst = isl_constraint_list_get_constraint(cst_list, j);
    
            for (int k = 0; k < isl_set_dim(set, isl_dim_out); k++)
            {
                std::string dim_name = isl_set_get_dim_name(set, isl_dim_out, k);
            
                if (isl_constraint_involves_dims(cst, isl_dim_set, k, 1))
                {
                    if (constraints_map.find(dim_name) != constraints_map.end())
                    {
                        constraints_map.at(dim_name) = constraints_map[dim_name] + 1;
                    }
                    else
                    {
                        constraints_map.insert({dim_name, 0});
                    }
                    break;
                }
            }
        }
    
    }
        return constraints_map;

    }

I tried to free each object (stil same problem) using the dedicated isl methods:

    isl_constraint_free
    isl_basic_set_free
    isl_
  • I don't see anywhere in that code that explicitly allocates or releases memory so unless you're running out of memory the problem is elsewhere. Time to start up the 'ol debugger and mull through the stack trace to narrow down where it starts from and try and determine what's causing it. Also stop using naked pointers and switch to `std::unique_ptr` and `std::shared_ptr` – Captain Obvlious Dec 21 '22 at 22:49
  • @CaptainObvlious Are you sure that these are all owning pointers? Putting non-owning pointers into `std::unique_ptr` or `std::shared_ptr` would create even more problems. That being said, isl seems to be primarily a C library so you might be right. Just remember to use the custom-destructor versions of the smart pointers. – paleonix Dec 21 '22 at 23:41
  • @paleonix I'm not saying stuff the existing naked pointers into `std::unique_ptr` or `std::shared_ptr` all willy nilly like. I'm saying stop using naked pointers altogether. – Captain Obvlious Dec 21 '22 at 23:45
  • @CaptainObvlious I would rather go with the C++ Core Guidelines which [allow raw pointers](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r2-in-interfaces-use-raw-pointers-to-denote-individual-objects-only) as long as they are non-owning and pointing to a single element (use span for multiple elements). If it is impossible to get a `nullptr`, references are a good alternative as well. I would imagine that ditching raw pointers is impossible when interfacing with C or old C++ libraries. – paleonix Dec 22 '22 at 09:44
  • @CaptainObvlious Or do you mean to write C++ wrappers for every "bad" library interface so that you don't have to touch raw pointers outside the wrappers? – paleonix Dec 22 '22 at 09:44
  • @paleonix Non-owning pointers do not *need* to be *naked* nor should they be. `T*` is a naked pointer and it's not readily clear to the reader whether it is non-owning or not. Both `owner` and `std::observer_ptr` (see the TS) are not naked pointers but are still non-owning and when used judiciously where `std::unique_ptr` and `std::shared_ptr` are not applicable makes it clear to the reader what the intent is. When interfacing to C or legacy C++ libraries something like `template owner = T` with `owner ownerPtr;` is sufficient to better express that non-ownership. – Captain Obvlious Dec 22 '22 at 16:44
  • @paleonix FWIW `owner` is described and recommended in the core guidelines you linked. – Captain Obvlious Dec 22 '22 at 16:56
  • @CaptainObvlious I think in terms of owning pointers we are totally in agreement, they should use RAII. My point (and the point of the Core Guidelines) is that if all owning pointers are wrapped, a naked pointer is always non-owning. I didn't know `std::experimental::observer_ptr` yet, it seems like a fine way of communicating intent. But saying that `owner` is non-owning (you mean it isn't RAII?) muddies the water I think. It has its name because it should signify ownership even if it doesn't enforce it. In the core guidelines it is listed as a bad example, not a good one. – paleonix Dec 22 '22 at 17:37
  • @CaptainObvlious See the fifth example below [P.6: What cannot be checked at compile time should be checkable at run time](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#p6-what-cannot-be-checked-at-compile-time-should-be-checkable-at-run-time). What it recommends for owning pointers are `unique_ptr` and `shared_ptr` in [GSL.owner: Ownership pointers](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#gslowner-ownership-pointers). My point was just that "no raw pointers" isn't necessarily consensus like "no raw owning pointers", even if it might not be a bad idea. – paleonix Dec 22 '22 at 17:43

0 Answers0