5

The following code is derived from a question here. (I am aware there are several logical problems with the code, but I do not understand how they are causing this problem.)

Edit: I am using CLANG on Windows, with common warning displayed for compile.

It generates a Divide by zero fault at the indicated line (first statement in the for loop.), but there are no apparent divides. Can anyone please provide some insight on why this error occurs?

Edit 2: Per comments: changing the 3rd argument in the sepia function from

void sepia(int height, int width, RGBTRIPLE image[height][width])

to

void sepia(int height, int width, RGBTRIPLE image[3][4])

eliminates the divide by zero error. Why?

typedef struct {
    double rgbtRed;
    double rgbtGreen;
    double rgbtBlue;
}RGBTRIPLE;

RGBTRIPLE image[3][4];

void sepia(int height, int width, RGBTRIPLE image[height][width])
{
    double sepiaRed = 0.0;
    double sepiaGreen = 0.0;
    double sepiaBlue = 0.0;
    // over height
    for (int h = 0; h < height; h++)
    {
        // over width
        for ( int w = 0; w < width; w++)
        {
            sepiaRed = .393 *  image[h][w].rgbtRed + .769 *  image[h][w].rgbtGreen + .189 *  image[h][w].rgbtBlue;
                           //  ^ Divide by zero occurs on this line.
            sepiaGreen = .349 *  image[h][w].rgbtRed + .686 *  image[h][w].rgbtGreen + .168 *  image[h][w].rgbtBlue;
            sepiaBlue = .272 *  image[h][w].rgbtRed + .534 *  image[h][w].rgbtGreen + .131 *  image[h][w].rgbtBlue;
            // space
            if (sepiaRed > 255 || sepiaGreen > 255 || sepiaBlue > 255)
            {
                sepiaRed = 255;
                sepiaGreen = 255;
                sepiaBlue = 255;
            }

            image[h][w].rgbtRed = (sepiaRed);
            image[h][w].rgbtBlue = (sepiaBlue);
            image[h][w].rgbtGreen = (sepiaGreen);
        }
    }
   return;
}

int main()
{
    sepia(3, 4, image);

    return 0;
}
ryyker
  • 22,849
  • 3
  • 43
  • 87
  • 9
    Works fine for me. What compiler and options are you using? Are you sure you are not mixing executables? Most probably you are running a executable that is out-of-sync with the source file. – KamilCuk Apr 27 '20 at 14:36
  • Compiler may generate division even if there's no explicit division. If you are using MSVC, maybe try `#pragma float_control(strict, on)` or `/fp:strict` – Alex Guteniev Apr 27 '20 at 14:38
  • @KamilCuk - I should have put that in the question, and will edit it in, but I am using CLANG with common warning displayed. – ryyker Apr 27 '20 at 14:38
  • is `image` initialized? – Mathieu Apr 27 '20 at 14:40
  • @ryyker What version of clang? – Thomas Jager Apr 27 '20 at 14:40
  • : / clang on linux also works. To be super super sure, post the assembly of the functions in the executable and then it's possible to read it instruction after instruction – KamilCuk Apr 27 '20 at 14:40
  • Have you looked at the assembler for the code to see where a divide instruction creeps in? – Jonathan Leffler Apr 27 '20 at 14:40
  • @KamilCuk - ...no, there is no other executable code, i.e. I have cleaned between builds after trying several variations, i.e. initializing the `image` before running etc. – ryyker Apr 27 '20 at 14:41
  • 1
    @Mathieu — the `image` array is global; it is zeroed. – Jonathan Leffler Apr 27 '20 at 14:41
  • 2
    Perhaps code is not handling the VLA correctly (div 0 coming from indexing) Try `RGBTRIPLE image[height][width]` --> `RGBTRIPLE image[3][4]` – chux - Reinstate Monica Apr 27 '20 at 14:41
  • Clang is complaining that `image` is being shadowed. Shouldn't matter in this case though, right? – Fiddling Bits Apr 27 '20 at 14:41
  • 1
    Unfortunately, I have not yet produced assembly with this compiler implementation. It is packaged in an NI ANSI C environment along with the default NI compiler, not sure I can even produce the assmbly. I will try the NI compiler, and also a GCC compiler I have to see if results are same. , if it exhibits the same behavior, I will post the assembly. – ryyker Apr 27 '20 at 14:44
  • @FiddlingBits - What does shadowed mean in this context? – ryyker Apr 27 '20 at 14:46
  • "default NI compiler" So are you not compiling in clang for Windows? – Thomas Jager Apr 27 '20 at 14:47
  • 1
    @ryyker Your local `image` in `sepia` is shadowing your global `image`. May be acceptable as you're using a pointer (array). – Fiddling Bits Apr 27 '20 at 14:47
  • @Chux - is your comment based on OP of linked, or the code I posted. I have adapted mine to make `image` global, and explicitly set the indexes as you have suggested. – ryyker Apr 27 '20 at 14:48
  • Observation based on code posted here. I assume the problem continues? – chux - Reinstate Monica Apr 27 '20 at 14:48
  • @ThomasJager - This version of CLANG is packaged with LabWIndows/CVI version 2019. The IDE does not indicate what version CLANG is included. – ryyker Apr 27 '20 at 14:50
  • @chux-ReinstateMonica - Your suggestion worked. Changing the argument from `image[height][width]` to `RGBTRIPLE image[3][4]` eliminated the error. – ryyker Apr 27 '20 at 14:53
  • @ThomasJager - No, I _am_ using the CLANG package that is delivered with NI LabWindows.CVI. I just do not know what version they adapted from. – ryyker Apr 27 '20 at 14:54
  • If running C99 or later , check `__STDC_NO_VLA__` "`__STDC_NO_VLA__` The integer constant 1, intended to indicate that the implementation does not support variable length arrays or variably modified types." – chux - Reinstate Monica Apr 27 '20 at 14:55
  • 1
    @FiddlingBits - I did try changing the variable name to eliminate shadowing, it had no affect. But as noted in comments under accepted answer, this behavior is likely a bug in NI's implementation of both of their compilers. Thanks. – ryyker Apr 27 '20 at 15:20
  • Just curious, does VLAs work when defined inside the function not as function parameters (something like [this](https://godbolt.org/z/vXmxsN), which I'm NOT suggesting, BTW)? – Bob__ Apr 27 '20 at 15:21

1 Answers1

3

Division by 0 due to array indexing.

Either VLA support is faulty or non-existent.

//                         VLA prototype         v-------------v           
void sepia(int height, int width, RGBTRIPLE image[height][width]) {
        //                      v----v
        sepiaRed = .393 *  image[h][w].rgbtRed + .769 *  ...

Code can use a non-VLA approach as below,

void sepia(int height, int width, RGBTRIPLE image[3][4]) {

VLA support begins with C99.

With C11 or later, check __STDC_NO_VLA__ for non-support.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • 1
    (+1) This certainly eliminated the divide by zero fault, but I can, and do use VLA in function body code (i.e. inside the `{...}` defining functions ) all the time. Odd that it would not be defined within the argument list, don't you think? – ryyker Apr 27 '20 at 15:01
  • 1
    @ryyker 1) What version of C is the compilation? Is `__STDC_NO_VLA__` defined? 2) VLA is one of those new-ish features I see compilers messing up. – chux - Reinstate Monica Apr 27 '20 at 15:04
  • I am using CLANG, packaged with NI LabWIndows/CVI version 2019, however do not know what Version of CLANG they adapted from . An I am set to C99 or better. I never select targets using anything older. – ryyker Apr 27 '20 at 15:06
  • 1
    @chux-ReinstateMonica C99 features are still new-ish? – Joseph Sible-Reinstate Monica Apr 27 '20 at 15:07
  • By the way, I just tried the same thing using the standard CVI compiler, and got same error. Is it possible (using the VLA arrangement.) that the values for `height` and `width` are not yet defined in the argument list, i.e. does C guaranteed the arguments are read in order from left to right? – ryyker Apr 27 '20 at 15:08
  • 1
    @ryyker Specifically, what is `__STDC_VERSION__`? Is `__STDC_NO_VLA__` defined? C99 not new-ish, but with C11 the optional-ness of VLA is the new-ish part. – chux - Reinstate Monica Apr 27 '20 at 15:08
  • 1
    @ryyker For a VLA in a prototype, `sepia(int height, int width, RGBTRIPLE image[height][width])` is fine. – chux - Reinstate Monica Apr 27 '20 at 15:09
  • @chux-ReinstateMonica - `__STDC_VERSION__` is `199901L` – ryyker Apr 27 '20 at 15:10
  • 1
    Hmmm, I suspect faulty compiler. Maybe one needs a later C99 compiler? – chux - Reinstate Monica Apr 27 '20 at 15:11
  • Yes, I agree its a possible (probable) bug. In particular because this behavior occurs in using both standard NI LabWindows/CVI compiler as well as their pre-installed CLANG optional compiler. I will notify NI as a potential bug. Thank you! – ryyker Apr 27 '20 at 15:13
  • 1
    @ryyker You might want to try un-shadow-ing. Use a function parameter name other than `image` to set that concern aside. – chux - Reinstate Monica Apr 27 '20 at 15:14
  • I will. That was also mentioned in the comments and I did not see it immediately. Thanks. – ryyker Apr 27 '20 at 15:15
  • @chux-ReinstateMonica - changing variable name to eliminate shadowing possibility did not fix. (but still should not have used same name in the first place. ) – ryyker Apr 27 '20 at 15:18
  • 1
    @ryyker Perhaps you can select a different standard with the `-std=` command-line option. Maybe it defaults to `c99` on your platform. – Ian Abbott Apr 27 '20 at 15:23
  • 1
    What I think is curious: Shouldn´t the compiler already complain about using a VLA at the definition in the argument list by throwing any error or warning instead of at its use in the expression later if the compiler does not support VLAs? – RobertS supports Monica Cellio Apr 27 '20 at 15:54
  • @RobertS supports Monica Cellio - I will register this as a bug to NI. At this point, there is no way to say this at all normal behavior. – ryyker Apr 27 '20 at 16:17
  • @RobertSsupportsMonicaCellio OP said "__STDC_VERSION__ is 199901L". As a C99 compliant compiler, support of VLA is required. I'd expect no error/warning. Yet compilers are not perfect. – chux - Reinstate Monica Apr 27 '20 at 16:17
  • Ok, a badly supported VLA leads to 0 indexes. But what part of the indexing operations is translated by the compiler with a division? – Roberto Caboni Apr 27 '20 at 16:43
  • @chux Yes, I saw that too but it obviously seems to not support VLAs although it should as a C99 compliant compiler. Therefore my thinking. So, technically it should support them although practically it doesn´t - Very interesting paradox. – RobertS supports Monica Cellio Apr 27 '20 at 16:58
  • @chux-ReinstateMonica - I was just going through your comment requests, I missed answering answering whether `__STDC_NO_VLA__` is defined. It is not. – ryyker Apr 27 '20 at 18:09
  • @ryyker `__STDC_NO_VLA__` came out in C11, so not suprising it is not defined with a C99 compiler. – chux - Reinstate Monica Apr 27 '20 at 19:02